433774 - <maction> handling of clicks should prevent events as needed

Status: RESOLVED WONTFIX

(Core :: MathML, defect)

Description

[Boris Zbarsky [:bzbarsky]]

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...

Comment 1

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

Attachment: testcase

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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.

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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.

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch

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>.

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch

Sorry for the spam, the previous patch doesn't include the new test.

Comment 6

[Karl Tomlinson (:karlt)]

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.)

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch

> 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.

Comment 8

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

(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 9

[Olli Pettay [:smaug][[e-mail]]]

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?

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch

Do you assume this way?

By this patch, the innermost <maction> is responsible to all outer <maction>s for handling the action.

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

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?

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(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.

Comment 13

[Olli Pettay [:smaug][[e-mail]]]

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.)

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Hmm, I tested on Opera, Chrome and Amaya. But nobody supports toggle action of <maction>... :-(

Comment 15

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

(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

Comment 16

[Olli Pettay [:smaug][[e-mail]]]

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 17

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 579989 [details] [diff] [review]
Patch

So, I think only the innermost <maction> should default handle the event.

Comment 18

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

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.

Comment 19

[Olli Pettay [:smaug][[e-mail]]]

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.

Comment 20

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

(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.

Comment 21

[Olli Pettay [:smaug][[e-mail]]]

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.

Comment 22

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

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.

Comment 23

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

FYI, maction toggle is now implemented in WebKit nightly. There is a reftest here:

http://trac.webkit.org/browser/trunk/LayoutTests/mathml/presentation/maction-toggle.html
http://trac.webkit.org/browser/trunk/LayoutTests/mathml/presentation/maction-toggle-expected.html

Comment 24

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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.

Comment 25

[Jonathan Watt [:jwatt]]

Mass bug change to replace various 'parity' whiteboard flags with the new canonical keywords. (See bug 1443764 comment 13.)

Comment 26

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

Mass WONTFIX for bugs related to legacy features that are not part of MathML Core (and with no plan to add them) or corresponding meta bugs. Please see bug 1495813 for the new meta bug.