Implement MathML DOM
Categories
(Core :: MathML, enhancement)
Tracking
()
Tracking | Status | |
---|---|---|
firefox71 | --- | fixed |
People
(Reporter: bkardell, Assigned: fredw)
References
(Blocks 4 open bugs, )
Details
(Keywords: dev-doc-complete)
Attachments
(2 files)
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) snap Chromium/76.0.3809.87 Chrome/76.0.3809.87 Safari/537.36
Steps to reproduce:
web platform tests for normalizing MathML elements
failed
Actual results:
They fail, they are historically an outlier without useful IDL.
Expected results:
We'd like them to pass
https://github.com/mathml-refresh/mathml/issues/83#issuecomment-518293228
Comment 1•5 years ago
|
||
We should probably land the fix for bug 1414372 first so we don't have to do extra IDL changes here after the fact.
Comment 2•5 years ago
|
||
We'd also need to add an actual MathMLElement interface.
And as an implementation note, we'd need to add tabindex parsing to nsMathMLElement::ParseAttribute
.
As a spec note, I don't see anything in https://mathml-refresh.github.io/mathml-core/#dom-and-javascript that has any mention of event handler content attributes (as opposed to IDL attributes), even though the tests test for that. The language at https://html.spec.whatwg.org/multipage/webappapis.html#event-handlers-on-elements,-document-objects,-and-window-objects is specific to HTML elements, obviously. We'd need to implement event handler content attributes as well to pass these tests, which is not a pure binding issue.
Note that the mapping from attribute names to event handler event types is NOT the same for HTML and SVG, by the way. If the intent is that MathML uses the HTML mapping, it should probably explicitly say that. I wish SVG actually specified its behavior too...
Comment 3•5 years ago
|
||
It's also possible that what should happen here is bugs blocking this one on the actual behavior changes (i.e. the handling of event handler attributes is a behavior change even if they don't get exposed via IDL), in addition to the IDL changes this bug would make.
Reporter | ||
Comment 4•5 years ago
|
||
We actually had an issue discussing some of this at https://github.com/mathml-refresh/mathml/issues/118. SVG, as I think you are saying, basically just says "you add/drop the 'on' and you have to support the global events. But, it sounds like you are saying they aren't the same in SVG? Can you explain a little more so that we know the right thing to do here? cc/amelia so maybe we can sort as much of this out as possible and un-weird these two
Comment 5•5 years ago
|
||
I see. I filed https://github.com/w3c/svgwg/issues/719 on the SVG bits.
Past that, Gecko maps the "onload" attribute in SVG to the "SVGLoad" event, not the "load" event. Similar for a number of other attributes. See https://searchfox.org/mozilla-central/rev/e62c920f7f6463239c6634113f8a8351e263b936/dom/svg/SVGElement.cpp#1374-1382. I just checked, and other browsers don't obviously seem to do that, so maybe we can drop it...
Comment 6•5 years ago
|
||
I guess that might be bug 620002.
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 7•5 years ago
|
||
This is now implemented in WebKit with the attribute names used in the WPT tests. Brian plans to update the MathML Core spec to make the names explicit.
Assignee | ||
Comment 8•5 years ago
|
||
Untested WIP patch.
Assignee | ||
Comment 9•5 years ago
|
||
@Brian: I think we also need tests to check the cases when onevent attributes are dynamically added/removed/modified.
Reporter | ||
Comment 10•5 years ago
|
||
(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #2)
We'd also need to add an actual MathMLElement interface.
And as an implementation note, we'd need to add tabindex parsing to
nsMathMLElement::ParseAttribute
.As a spec note, I don't see anything in https://mathml-refresh.github.io/mathml-core/#dom-and-javascript that has any mention of event handler content attributes (as opposed to IDL attributes), even though the tests test for that. The language at https://html.spec.whatwg.org/multipage/webappapis.html#event-handlers-on-elements,-document-objects,-and-window-objects is specific to HTML elements, obviously. We'd need to implement event handler content attributes as well to pass these tests, which is not a pure binding issue.
Note that the mapping from attribute names to event handler event types is NOT the same for HTML and SVG, by the way. If the intent is that MathML uses the HTML mapping, it should probably explicitly say that. I wish SVG actually specified its behavior too...
(also re comment#5)
bz, speaking to amelia's reasoning for doing it the way they did on the linked svg issue - was just talking with her about how we manage to not explicitly list and be explicit enough. Would something as simple as the following phrasing work?
"All MathML elements support event handler content attributes, as described in HTML. All event handler content attributes noted by HTML as being supported by all HTMLElements are supported by all MathML elements as well, and reified as the relevant event handler IDL attributes as described in that specification, and defined in the MathMLElement IDL. "
Comment 11•5 years ago
|
||
The link to https://html.spec.whatwg.org/multipage/webappapis.html#event-handler-contentattributes you have there is broken, no? Presumably it should link to https://html.spec.whatwg.org/multipage/webappapis.html#event-handler-content-attributes.
Past that, the "noted by HTML as being supported by all HTML elements" (note spelling of that last term!) text should probably link to https://html.spec.whatwg.org/multipage/webappapis.html#event-handlers-on-elements,-document-objects,-and-window-objects. In practice the intent is to pull in that first big list and oncut/oncopy/onpaste, right?
I don't know that the "reified as ..." bit is needed, as long as the IDL links to the right places (which presumably it does given that presumably it just pulls in the DocumentAndElementEventHandlers
and GlobalEventHandlers
mixins). I'm not sure whether this "reified as ..." bit is actively harmful, but it did confuse me a bit, since the actual reification, if there is one, is in terms of "event handler" structs, and the content and IDL attributes are just ways to create and modify those structs; neither one is really the full source of truth.
Assignee | ||
Comment 12•5 years ago
|
||
@Boris: Can you please take a look at the preliminary rename-only patch https://phabricator.services.mozilla.com/D45128 ? Thanks!
Comment 13•5 years ago
|
||
Yes, it's in my review queue... Will get to it soon.
Assignee | ||
Comment 14•5 years ago
|
||
I uploaded a new patch. I think the implementation is essentially done, but we need to wait for the latest WPT changes, to send the intent-to and perhaps to write more tests for the tabIndex/focus stuff.
Brian has added more details to the MathML Core spec:
https://mathml-refresh.github.io/mathml-core/#event-handler-content-attributes
https://mathml-refresh.github.io/mathml-core/#focus
Assignee | ||
Comment 15•5 years ago
|
||
I wrote the intent to prototype: https://groups.google.com/forum/#!topic/mozilla.dev.platform/ssTytf-pT7k
and sent the patch for a first review.
Assignee | ||
Comment 16•5 years ago
|
||
There has not been any complaint on the "intent to" email since I sent it one week ago. I'd like to get the MathML DOM changes landed so that we are closer to WebKit in https://mathml-refresh.github.io/mathml-core/implementation-report.html
@Boris: Can you please take a look at attachment 9091079 [details]?
IIUC, these are the main things I were not sure about:
- Whether we need a runtime flag.
- Whether we need a EventNameType_MathML flag that would basically duplicate the EventNameType_HTML one.
- Whether we can factor out shared logic between SVG & HTML and use it for MathML too (I guess yes, but that's probably something to do in a separate bug).
- Whether we need more tests for the tabindex/focus related changes.
Thanks!
Comment 17•5 years ago
|
||
@Boris: Can you please take a look at attachment 9091079 [details]?
Yes, I have been since Friday. I was just off yesterday...
Whether we need a runtime flag.
How likely is compat fallout? If we were putting things behind a flag here, which things would go behind the flag?
Whether we need a EventNameType_MathML flag that would basically duplicate the EventNameType_HTML one.
The only reason the EventNameType
thing exists is for purposes of implementing IsEventAttributeNameInternal
. If you can get the right behavior without adding a new one, I wouldn't introduce a new one.
Whether we can factor out shared logic between SVG & HTML and use it for MathML too (I guess yes, but that's probably something to do in a separate bug).
That seems reasonable. Some such logic is already factored into nsStyledElement
. We could either move move things into there or add another class below or above it. Followup for this is ok, though filing the followup now, with a list of things that can be shared, might not be a bad idea.
Whether we need more tests for the tabindex/focus related changes.
Well, the tabindex bits definitely need more tests, since they're buggy... ;)
Assignee | ||
Comment 18•5 years ago
|
||
(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #17)
How likely is compat fallout? If we were putting things behind a flag here, which things would go behind the flag?
As I mentioned on the mailing list, I personally don't think a flag is necessary. People currently expect elements to have the same IDL attributes as HTML elements (e.g. bug 1231085 and bug 1530110 for style) so we are really just normalizing MathML's behavior. Apple reviewers were ok to land WebKit support without a runtime flag. Also, I'm not exactly sure what we should/can put under a flag. Anyway, if someone really feels we should, this can be reconsidered (I didn't see any complaints on the intent to ship).
Assignee | ||
Comment 19•5 years ago
|
||
It looks like more test updates are needed before landing this:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d750689827750b7e7211a3bfb1a39cd94010f053
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Comment 20•5 years ago
|
||
Pushed by nbeleuzu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/718ef1098af5
Implement MathML DOM r=bzbarsky
Comment 23•5 years ago
|
||
Backed out changeset 718ef1098af5 (Bug 1571487) for causing perma wpt merges in /mathml/relations/html5-tree/tabindex-002.html CLOSED TREE
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=269783425&repo=autoland&lineNumber=3818
Comment 25•5 years ago
|
||
Assignee | ||
Comment 26•5 years ago
|
||
I only see mathml/relations/html5-tree/tabindex-002.html failing on mac. The test passes on Linux.
(This test is basically copied from html/interaction/focus/sequential-focus-navigation-and-the-tabindex-attribute/focus-tabindex-order.html but that one does not have any failure expectation.)
@Henrik: Are you aware of any particular issue with marionette/testdriver/send_keys? I can't find anything on Bugzilla.
Comment 27•5 years ago
|
||
Focus model is different on mac than on linux / windows. Probably the test assumes the linux / windows focus model.
Comment 28•5 years ago
|
||
In particular, iirc, on mac only buttons and inputs are tab-navigable by default, or something like that...
Comment 29•5 years ago
|
||
Assignee | ||
Comment 30•5 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #28)
In particular, iirc, on mac only buttons and inputs are tab-navigable by default, or something like that...
Thanks Emilio. I tried to do a similar test using only HTML (no MathML) and indeed on macOS links are not tab-navigable by default, so the test fails...
(see also the related https://github.com/Igalia/chromium-dev/issues/50)
@Boris: Not sure what would be the easiest way to move forward? Add a failure expectation on mac? Do not test navigation of mathml link? Find a way to force the link to be tab-navigable?
Assignee | ||
Updated•5 years ago
|
Comment 31•5 years ago
|
||
Well, the test is fundamentally buggy, since it assumes things that are simply not true, right? Tests are meant to test the things specs actually require, not whatever happens to work on one particular setup or operating system. Specs do not require links to be tab-navigable.
So the right answer in general is to fix the test so it's not testing non-spec-required things. Depending on the purpose of the test that could mean removing the test, or removing just the text6
bit from the test, or having it test that tab-navigability of MathML links matches tab-navigability of HTML links, or something else. I don't know what the exact goal of this test is, so hard to say what it should test.
Short-term you can add a failure expectation on Mac, yes. I'd probably go that way. You could likely set the accessibility.tabfocus
pref to 7
to override the default Mac behavior, but I would prefer we test the configurations we actually ship. Though it might not be terrible to have a separate test that makes sure that preference works as it should, of course. I see no such tests right now.... That's way out of the scope of this bug, though. File a followup for that, please?
Assignee | ||
Comment 32•5 years ago
|
||
OK, I don't really know what's the rule for links I just learned today that they can be not tab-navigable :-)
I opened https://github.com/mathml-refresh/mathml/issues/152 and will mark this as failing on mac for now
Assignee | ||
Comment 33•5 years ago
|
||
Comment 34•5 years ago
|
||
Pushed by btara@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/30154d163aca
Implement MathML DOM r=bzbarsky
Comment 36•5 years ago
|
||
bugherder |
Comment 38•5 years ago
|
||
Mentioned this on Fx71 for developers:
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/71#MathML
And also on the MathML element reference:
https://developer.mozilla.org/en-US/docs/Web/MathML/Element
The MathMLElement class is documented here:
https://developer.mozilla.org/en-US/docs/Web/API/MathMLElement
Description
•