[webkit-reviews] review denied: [Bug 120058] Add an MathMLSelectElement class to implement <maction> and <semantics> : [Attachment 216535] Patch V9

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has denied	review:
Bug 120058: Add an MathMLSelectElement class to implement <maction> and
<semantics>
https://bugs.webkit.org/show_bug.cgi?id=120058

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

------- Additional Comments from Brent Fulgham <bfulgham at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=216535&action=review


I think this is ready to land, modulo a couple of minor tweaks (see below).

> LayoutTests/mathml/presentation/semantics.html:18
> +    <!-- This is not valid MathML but has become a standard hack to include
SVG in MathML because of Gecko's initial implementation. -->

Please mark this test as invalid in TestExpectations, and reference bug 124128
for the failure.

> Source/WebCore/ChangeLog:11
> +	   This adds a new MathMLSelectElement class to prepare the
implementation of the <maction> and <semantics> elements, for which only one
"selected" child is visible. We now simply display the first child of the
<semantics> element instead of hiding the annotations and this allows to handle
the use case of SVG-in-MathML as generated by Instiki ; Gecko's selection
algorithm will be implemented later (bug 100626). We now also rely on the
@actiontype and @selection attributes to select the visible <maction> child ;
It remains to deal with the user interaction (bug 85734).

For some reason this appears as a single long line in the ChangeLog. This
should be broken up into standard lines.

> Source/WebCore/mathml/MathMLSelectElement.cpp:85
> +	       // FIXME: implement user interaction for the "tooltip",
"statusline" and "toggle" action types. See bug 85734.

Please include the Bug number to resolve the FIXME.