[webkit-reviews] review granted: [Bug 25259] AX: VoiceOver cmd-cntl-space does not follow the google.com->more link : [Attachment 29569] patch to fix accessibility problem with invisible elements

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted chris fleizach
<cfleizach at apple.com>'s request for review:
Bug 25259: AX: VoiceOver cmd-cntl-space does not follow the google.com->more
link
https://bugs.webkit.org/show_bug.cgi?id=25259

Attachment 29569: patch to fix accessibility problem with invisible elements
https://bugs.webkit.org/attachment.cgi?id=29569&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +	       bool visibilityChanged = (m_style->visibility() !=
newStyle->visibility() ||
> +					       m_style->zIndex() !=
newStyle->zIndex() ||
> +					       m_style->hasAutoZIndex() !=
newStyle->hasAutoZIndex());

The formatting here retains strange indenting and parentheses from the if
statement. I would use this format:

    bool visibilityChanged = m_style->visibility() != newStyle->visibility()
	|| m_style->zIndex() != newStyle->zIndex()
	|| m_style->hasAutoZIndex() != newStyle->hasAutoZIndex();

Partly this is because I prefer the "operators at beginning of line" style
because I think it makes it clearer that subsequent lines are continuations of
the expression. And partly because I think the entire expression is easier to
read if we don't do excessive indentation.

r=me