The files other than BufferedRenderer LGTM. For BufferedRenderer it would help to have a header ...
17 years, 4 months ago
(2008-11-23 05:06:40 UTC)
#2
The files other than BufferedRenderer LGTM.
For BufferedRenderer it would help to have a header comment to describe what the
semantics to be maintained by the class are. In particular since comments are
being handed to the renderer, is it the responsibility of the renderer or of an
earlier pass to remove browser specific comment pragma?
http://codereview.appspot.com/9662/diff/239/251
File src/com/google/caja/render/BufferingRenderer.java (right):
http://codereview.appspot.com/9662/diff/239/251#newcode125
Line 125:
This method does not correctly handle IE's /*@cc_on ... */ blocked comments -
for example the EOL removal is not semantics preserving if the comment block is
really a conditional compiler block containing javascript. Either the renderer
should de-sting all s/@/ /g in comments or drop comments containing those blocks
entirely.
Added a comment to BufferedRenderer.consume, and another to the class itself. On 2008/11/23 05:06:40, jasvir ...
17 years, 4 months ago
(2008-11-24 22:15:50 UTC)
#3
Added a comment to BufferedRenderer.consume, and another to the class itself.
On 2008/11/23 05:06:40, jasvir wrote:
> The files other than BufferedRenderer LGTM.
>
> For BufferedRenderer it would help to have a header comment to describe what
the
> semantics to be maintained by the class are. In particular since comments are
> being handed to the renderer, is it the responsibility of the renderer or of
an
> earlier pass to remove browser specific comment pragma?
>
> http://codereview.appspot.com/9662/diff/239/251
> File src/com/google/caja/render/BufferingRenderer.java (right):
>
> http://codereview.appspot.com/9662/diff/239/251#newcode125
> Line 125:
> This method does not correctly handle IE's /*@cc_on ... */ blocked comments -
> for example the EOL removal is not semantics preserving if the comment block
is
> really a conditional compiler block containing javascript. Either the
renderer
> should de-sting all s/@/ /g in comments or drop comments containing those
blocks
> entirely.
Issue 9662: Reimplement pretty printer and minimizer on top of BufferedRenderer
(Closed)
Created 17 years, 4 months ago by MikeSamuel
Modified 16 years, 8 months ago
Reviewers: Jasvir, ihab.awad
Base URL: http://google-caja.googlecode.com/svn/trunk/
Comments: 1