[webkit-reviews] review denied: [Bug 106924] [GTK] GTK does not expose heading level correctly. Was: accessibility/heading-level.html is failing : [Attachment 212466] Patch proposal

[bugzilla-daemon at webkit.org]

chris fleizach <cfleizach at apple.com> has denied Mario Sanchez Prada
<mario at webkit.org>'s request for review:
Bug 106924: [GTK] GTK does not expose heading level correctly. Was:
accessibility/heading-level.html is failing
https://bugs.webkit.org/show_bug.cgi?id=106924

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

------- Additional Comments from chris fleizach <cfleizach at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=212466&action=review


r- for some style considerations. thanks

> Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:618
> +	   GValue value = { 0, { { 0 } } };

can you use G_VALUE_INIT here?

> Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:621
> +	       return 0.0f;

style guidelines say not to append "f" to literals

> Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:632
> +	   if (!ok)

this should be if (ok)... then you can let the failure case fall through to the
final return value

> Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:633
> +	       return 0.0f;

ditto

> Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:636
> +    return 0.0f;

ditto

> Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp:789
> +	       return 0.0f;

ditto about .f

> Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp:799
> +	   double headingLevelValue = headingLevel.toDouble(&ok);

ditto about comments from DRT