[webkit-reviews] review granted: [Bug 125756] [CSSRegions] Incorrect repaint of fixed element with transformed parent : [Attachment 219425] Patch 2

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Mihnea Ovidenie <mihnea at adobe.com>'s
request for review:
Bug 125756: [CSSRegions] Incorrect repaint of fixed element with transformed
parent
https://bugs.webkit.org/show_bug.cgi?id=125756

Attachment 219425: Patch 2
https://bugs.webkit.org/attachment.cgi?id=219425&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=219425&action=review


> Source/WebCore/rendering/FlowThreadController.cpp:290
> +// The list of layers is returned already sorted by z-Index.

I think the word “already” only makes sense here if you know how the code used
to work and so should be removed. The “I” in “z-index” should not be
capitalized.

> Source/WebCore/rendering/FlowThreadController.cpp:301
> +	   // Iterate over the lists of fixed positioned layers collected at
flow thread layer level

I don’t understand how this comment adds to the readability of the code. A for
loop already says “iterate”. The if statements already says “fixed position”
and the variable name also says “fixed positioned layers”, albeit with
“positioned” abbreviated to “pos”. The other variable name says “flow thread
layer”. The comment just repeats exactly what the code says. Please omit such
comments.

> Source/WebCore/rendering/FlowThreadController.cpp:323
> +    // Sort the fixed layers list

Same thing about this comment. Fine to write this as a placeholder before
writing the code, but once you write the code the comment seems superfluous.