Bug 139403 – The display property of many MathML elements can not be overriden by page authors

• Component: MathML
• Status: RESOLVED FIXED

[Frédéric Wang (:fredw)]

Created attachment 242819 [details]
Testcase

The display property of many MathML elements can not be overriden by page authors. This is because they have some

display: ...flex... !important;

property in the mathml.css user agent stylesheet. See the attached testcase.

I guess we can not just remove the "!important" rules, as the MathML code assumes that the elements are really flexboxes...

[Frédéric Wang (:fredw)]

We should consider removing the !important property after bug 153991.

[Frédéric Wang (:fredw)]

Created attachment 282878 [details]
Patch

[Frédéric Wang (:fredw)]

Created attachment 283065 [details]
Patch

[Build Bot]

Comment on attachment 283065 [details]
Patch

Attachment 283065 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/1643287

New failing tests:
accessibility/roles-computedRoleString.html

[Build Bot]

Created attachment 283078 [details]
Archive of layout-test-results from ews101 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 283065 [details]
Patch

Attachment 283065 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/1643354

New failing tests:
accessibility/roles-computedRoleString.html

[Build Bot]

Created attachment 283082 [details]
Archive of layout-test-results from ews113 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 283065 [details]
Patch

Attachment 283065 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/1643469

New failing tests:
accessibility/roles-computedRoleString.html

[Build Bot]

Created attachment 283088 [details]
Archive of layout-test-results from ews104 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Frédéric Wang (:fredw)]

Created attachment 283145 [details]
Patch

[Build Bot]

Comment on attachment 283145 [details]
Patch

Attachment 283145 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/1646843

New failing tests:
accessibility/roles-computedRoleString.html

[Build Bot]

Created attachment 283151 [details]
Archive of layout-test-results from ews100 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 283145 [details]
Patch

Attachment 283145 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/1646844

New failing tests:
accessibility/roles-computedRoleString.html

[Build Bot]

Created attachment 283152 [details]
Archive of layout-test-results from ews105 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 283145 [details]
Patch

Attachment 283145 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/1647115

New failing tests:
accessibility/roles-computedRoleString.html

[Build Bot]

Created attachment 283162 [details]
Archive of layout-test-results from ews113 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Joanmarie Diggs]

Ah. Test noise resulting in failures that are not failures.

IMHO, this is a case where the right thing to do is to eliminate the noise rather than updating the noise. An example of where this is done for the full test, rather than on an element-by-element basis, can be found here:
https://trac.webkit.org/browser/trunk/LayoutTests/accessibility/w3c-svg-roles.html#L180

[Frédéric Wang (:fredw)]

Created attachment 283164 [details]
Patch

[Joanmarie Diggs]

(In reply to comment #18)
> Created attachment 283164 [details]
> Patch

The accessibility test has been modified to not dump the noise. As a result, there should no longer be a test failure or different expectations needed.
https://trac.webkit.org/changeset/202994

[Manuel Rego Casasnovas]

Comment on attachment 283164 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=283164&action=review

Nice patch Fred, I also believe we should get rid of that !important rules.

Informal r+ here.

> LayoutTests/imported/mathml-in-html5/mathml/relations/css-styling/display-1-expected.html:9
> +  <div style="background: green; color: red; width: 200px; height: 200px;">

Nit: This is an imported test so I'm fine with it. We should fix the test in the suite, but "color: red" is not needed here.

> LayoutTests/imported/mathml-in-html5/mathml/relations/css-styling/display-1.html:12
> +  <div style="background: green; color: red; width: 200px; height: 200px;">

Ditto.

> Source/WebCore/ChangeLog:7
> +        'display: block'. We remove the !important property so that users can override the display

Nit: "!important rule" or "!important keyword" sounds better than "property" to me.

[Frédéric Wang (:fredw)]

Created attachment 283419 [details]
Patch

[Frédéric Wang (:fredw)]

@Brent: Can you please have a look at the new patch?

[Brent Fulgham]

Comment on attachment 283419 [details]
Patch

r=me

[Frédéric Wang (:fredw)]

Committed r203163: <http://trac.webkit.org/changeset/203163>