Open Bug 433774 Opened 17 years ago Updated 2 years ago

<maction> handling of clicks should prevent events as needed

Categories

(Core :: MathML, defect)

x86
Linux
defect

Tracking

()

People

(Reporter: bzbarsky, Unassigned)

References

Details

(Keywords: parity-safari)

Attachments

(2 files, 3 obsolete files)

It seems that if to <mactions> that toggle are nested, a click on content inside the inner one toggles the outer one. The spec is pretty vague as to what the "right" behavior is, so this might need clarification. But it would make some sense to make the <maction> toggle a default action and preventDefault the event once it happens or something like that...
Blocks: maction
Attached file testcase (obsolete) (deleted) —
The testcase doesn't make sense since click events fired on <mn> rather than <maction>. It's easy to check defaultPrevented in nsMathMLmactionFrame.cpp. However, there is an issue. Should ignore the events whether only when the click event target is a child of the <maction> or not. I.e., whether includes descendant <mn>s or not.
http://www.w3.org/TR/MathML3/chapter3.html#presm.maction > When a MathML application receives a mouse event that may be processed by two or more nested maction elements, the innermost maction element of each action type should respond to the event. Assuming that *only* the innermost maction handles the events in following case: <maction actiontype="toggle" id="maction_parent" selection="1"> <maction actiontype="toggle" id="maction_child" selection="1"> <mn>1</mn> <mn>2</mn> </maction> <mn>3</mn> </maction> the values will be: first click second click rendered: 2 1 selection of parent: 1 1 selection of child: 2 1 So, 3 is never appeared, it shouldn't be intentional behavior of spec editor. Therefore, we should handle both <maction>s. (This is current Gecko's behavior) If web apps only prevented click events whose target was parent's children, the result would be strange. I think that it's impossible to implement the intentional behavior of the testcase by preventing default.
Attached patch Patch (obsolete) (deleted) — Splinter Review
See the test's comment and above comments for nested <maction>'s behavior. Consumed click events shouldn't toggle its content even if the event target is another <maction>'s <mn>.
Assignee: nobody → masayuki
Attachment #560347 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #578196 - Flags: review?(karlt)
Attachment #578196 - Flags: review?(bugs)
Attached patch Patch (obsolete) (deleted) — Splinter Review
Sorry for the spam, the previous patch doesn't include the new test.
Attachment #578196 - Attachment is obsolete: true
Attachment #578197 - Flags: review?(karlt)
Attachment #578197 - Flags: review?(bugs)
Attachment #578196 - Flags: review?(karlt)
Attachment #578196 - Flags: review?(bugs)
Comment on attachment 578197 [details] [diff] [review] Patch This looks correct, thanks. I'm curious about what calls preventDefault() when the inner maction is processed (so that the outer maction sees GetDefaultPrevented as true)? >+ synthesizeMouseAtCenter(parent, {}); // expect "3" is rendered >+ >+ is(parent.getAttribute("selection"), "2", >+ "parent maction's selection attribute isn't 2"); >+ is(child.getAttribute("selection"), "2", >+ "child maction's selection attribute isn't 2"); >+ synthesizeMouseAtCenter(parent, {}); // expect "3" is rendered >+ >+ is(parent.getAttribute("selection"), "2", >+ "parent maction's selection attribute isn't 2"); >+ is(child.getAttribute("selection"), "1", >+ "child maction's selection attribute isn't 1"); >+ synthesizeMouseAtCenter(parent, {}); // expect "3" is rendered >+ >+ is(parent.getAttribute("selection"), "2", >+ "parent maction's selection attribute isn't 2"); >+ is(child.getAttribute("selection"), "2", >+ "child maction's selection attribute isn't 2"); >+ >+ synthesizeMouseAtCenter(parent, {}); // expect "3" is rendered >+ >+ is(parent.getAttribute("selection"), "2", >+ "parent maction's selection attribute isn't 2"); >+ is(child.getAttribute("selection"), "2", >+ "child maction's selection attribute isn't 2"); Looks like the comments here should be removed. (They don't match the "is" tests, and the "is" tests already document what is expected.)
Attachment #578197 - Flags: review?(karlt) → review+
Attached patch Patch (deleted) — Splinter Review
> I'm curious about what calls preventDefault() when the inner maction is processed (so that the outer maction sees GetDefaultPrevented as true)? Yes. Now, <maction> listens to the events *after* all content's event handlers handle the events. All nodes which are between root node and event target element can prevent the default action of <maction>s which are between the root node and the event target.
Attachment #578197 - Attachment is obsolete: true
Attachment #578478 - Flags: review?(bugs)
Attachment #578197 - Flags: review?(bugs)
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #3) > http://www.w3.org/TR/MathML3/chapter3.html#presm.maction > > When a MathML application receives a mouse event that may be processed by two or more nested maction elements, the innermost maction element of each action type should respond to the event. Maybe one should report to the W3C Math WG that the description of maction behavior is not really consistent with what is done elsewhere regarding how click events are dealt.
Comment on attachment 578478 [details] [diff] [review] Patch So it is up to web page to call preventDefault? I think that change is ok, but perhaps not enough. would it make sense to set NS_EVENT_FLAG_PREVENT_MULTIPLE_ACTIONS when handling click, and also check that NS_EVENT_FLAG_PREVENT_MULTIPLE_ACTIONS isn't set before handling click. Or how should this all work if there is <html:a> somewhere in ancestor chain?
Attachment #578478 - Flags: review?(bugs) → review-
Attached patch Patch (deleted) — Splinter Review
Do you assume this way? By this patch, the innermost <maction> is responsible to all outer <maction>s for handling the action.
Attachment #579989 - Flags: review?(bugs)
Comment on attachment 579989 [details] [diff] [review] Patch > nsMathMLmactionFrame::MouseListener::HandleEvent(nsIDOMEvent* aEvent) > { >+ bool defaultPrevented = true; >+ aEvent->GetDefaultPrevented(&defaultPrevented); >+ if (defaultPrevented) { >+ return NS_OK; >+ } >+ >+ // Don't handle the event if multiple actions handling is prevented. >+ nsCOMPtr<nsIPrivateDOMEvent> privateEvent = do_QueryInterface(aEvent); >+ nsEvent* event = nsnull; >+ if (privateEvent) { >+ event = privateEvent->GetInternalNSEvent(); >+ if (event && (event->flags & NS_EVENT_FLAG_PREVENT_MULTIPLE_ACTIONS)) { >+ return NS_OK; >+ } >+ event->flags |= NS_EVENT_FLAG_PREVENT_MULTIPLE_ACTIONS; >+ } This is what I was thinking. >+ for (nsIContent* content = mOwner->GetContent(); content; >+ content = content->GetParent()) { >+ nsMathMLmactionFrame* mactionFrame = >+ do_QueryFrame(content->GetPrimaryFrame()); >+ if (!mactionFrame) { >+ continue; >+ } >+ >+ if (eventType.EqualsLiteral("mouseover")) { >+ mactionFrame->MouseOver(); >+ } else if (eventType.EqualsLiteral("click")) { >+ mactionFrame->MouseClick(); >+ } else if (eventType.EqualsLiteral("mouseout")) { >+ mactionFrame->MouseOut(); >+ } else { >+ NS_ABORT(); >+ } > } But I don't understand this. Is there a reason to handle the event in more than one maction?
(In reply to Olli Pettay [:smaug] from comment #11) > >+ for (nsIContent* content = mOwner->GetContent(); content; > >+ content = content->GetParent()) { > >+ nsMathMLmactionFrame* mactionFrame = > >+ do_QueryFrame(content->GetPrimaryFrame()); > >+ if (!mactionFrame) { > >+ continue; > >+ } > >+ > >+ if (eventType.EqualsLiteral("mouseover")) { > >+ mactionFrame->MouseOver(); > >+ } else if (eventType.EqualsLiteral("click")) { > >+ mactionFrame->MouseClick(); > >+ } else if (eventType.EqualsLiteral("mouseout")) { > >+ mactionFrame->MouseOut(); > >+ } else { > >+ NS_ABORT(); > >+ } > > } > But I don't understand this. Is there a reason to handle the event in more > than one maction? See the testcase in the patch. <maction actiontype="toggle"> <maction actiontype="toggle"> <mn>1</mn> <mn>2</mn> </maction> <mn>3</mn> </maction> First, "1" is rendered. If click on "1", both <maction> toggles the selection. Then, "2" should be selected in inner <maction> and "3" should be selected in outer <maction> and "3" should be rendered. Next, click on "3", only outer <maction> should toggle the selection, then, "2" should be rendered. So, when innter <maction> handles the event, outer one should handle it too. If this is invalid behavior, "3" is never rendered by click. I don't think this behavior is the intent of the spec but I'm not sure.
I don't understand why clicking 1 should toggle the outer <maction>, if it toggles the inner <maction>. To me this is similar case as nested <a> elements. Only the innermost should handle the click. What do other browsers do in this case? (I don't remember which browsers support MathML.)
Hmm, I tested on Opera, Chrome and Amaya. But nobody supports toggle action of <maction>... :-(
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #14) > Hmm, I tested on Opera, Chrome and Amaya. But nobody supports toggle action > of <maction>... :-( You may want to try with MathJax. Just add the following code into the head section of the Web page you want to test: <script type="text/javascript" src="http://cdn.mathjax.org/mathjax/latest/MathJax.js?config=MML_HTMLorMML"> </script> and choose the MathJax output with Right click on a MathML formula => Settings => Math Renderer => HTML-CSS
So, "When a MathML application receives a mouse event that may be processed by two or more nested maction elements, the innermost maction element of each action type should respond to the event. " says that only the inner most maction should handle the event. Or that is how I interpret it. ("each action type" is strange though)
Comment on attachment 579989 [details] [diff] [review] Patch So, I think only the innermost <maction> should default handle the event.
Attachment #579989 - Flags: review?(bugs) → review-
So basically, if we have the following nested structure: <maction id="maction1" actiontype="tooltip"> <maction id="maction2" actiontype="toggle"> <maction id="maction3" actiontype="tooltip"> <maction id="maction4" actiontype="toggle"> <maction id="maction5" actiontype="toggle"> <mi>x</mi> ... </maction> </maction> </maction> </maction> </maction> and we click on the "x", only maction5 and maction3 should respond to the event (the innermost maction element of each action type should respond to the event) according to the spec.
Yeah, that is what the spec says, but I'm not quite sure it makes sense - though I'm certainly not very familiar with MathML. But I would accept a patch which does what the spec says.
(In reply to Olli Pettay [:smaug] from comment #19) > Yeah, that is what the spec says, but I'm not quite sure it makes sense - > though I'm certainly > not very familiar with MathML. > But I would accept a patch which does what the spec says. I was just trying to explain how I interpret this "each action type". However I don't really think it makes sense either, I would rather suggest to keep consistency with how we implement event handling in other languages such as HTML.
Either way. We can implement what the spec says, and ask the spec to be changed, (and fix the implementation later) or we can implement what we think the spec should say and inform the MathML WG about the bug in the spec attachment 579989 [details] [diff] [review] doesn't implement the spec, nor the preferred handling.
FYI, I mentioned this issue in a series of questions related to the maction element: http://lists.w3.org/Archives/Public/www-math/2012Jun/0001.html CC'ing Andrii, who is working on the <maction> element during his Google Summer of Code project.
Flags: needinfo?(masayuki)
Whiteboard: [parity-webkit]
Thank you for the information. However, I'm busy for other work now. If somebody want to work on this, feel free to take this and modify my patch.
Assignee: masayuki → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(masayuki)
Mass bug change to replace various 'parity' whiteboard flags with the new canonical keywords. (See bug 1443764 comment 13.)
Keywords: parity-safari
Whiteboard: [parity-webkit]
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: