533845 - Mouse events don't work in content iframe, if iframe is inside a xul panel

Status: RESOLVED FIXED

(Core :: Web Painting, defect, P2)

Description

[Jeff Muizelaar [:jrmuizel]]

I'm not sure exactly what's wrong but this was discussed in a meeting

Comment 1

[Myk Melez [:myk] [@mykmelez]]

The specific problem is that content iframes in XUL panels don't respond to mouse events.  The XUL panel docs <https://developer.mozilla.org/en/XUL%3apanel> say that this is because of bug 130078.  But roc thinks this might be a separate bug that is easier to fix than that one.

This bug is to track the separate fix, since it might enable us to implement Jetpack bug 494238.

roc: is this something were going to look at, or were you going to get someone else to take a look at it?

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Olli could look at it.

Comment 3

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

A *minimal* testcase would be very useful!

Comment 4

[Myk Melez [:myk] [@mykmelez]]

Attachment: minimal testcase

Here's a minimal testcase.  Save this locally and start it with:

firefox -chrome file:///path/to/533845-testcase.xul

Then try to interact with the page in the content iframe contained within the XUL panel.  You should be able to, but you can't: clicks have no effect (even the scrollbar is unresponsive), and typing doesn't do anything.

Comment 5

[Myk Melez [:myk] [@mykmelez]]

Olli: have you made any progress figuring this out?

Comment 6

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

Sorry, no.
Assigning to myself so that I might remember/prioritize this.

Comment 7

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

Attachment: patch

Myk, want to test this one?
I'm writing mochitest for this.

Comment 8

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

Note, the fix is for mouse events.

Comment 9

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

Though, key events seem to work usually, but not always.
Not sure what the problem with key events is. Something to do with focus.

Comment 10

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

Ok, if I manually focus iframe content (window.content.focus()) right after opening the panel, key events work just fine.

Comment 11

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

Ok, lets keep this bug for mouse event handling.

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

+              (!doc || !popup->GetContent() ||
+               doc == popup->GetContent()->GetOwnerDoc() ||

Do you need these?

I think !doc and !popup->GetContent() can't possibly be true. If doc == popup->GetContent()->GetOwnerDoc() then !nsContentUtils::ContentIsCrossDocDescendantOf(doc, popup->GetContent()) must also be true.

Comment 13

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

if doc ==
popup->GetContent()->GetOwnerDoc() then
nsContentUtils::ContentIsCrossDocDescendantOf(doc, popup->GetContent()) returns true. And !... is false.
And I want to keep the common case where frame and popup are
both from the same document fast.

!doc check is useless sure. And probably also the !popup-GetContent() check.

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I don't get it. If doc == popup->GetContent()->GetOwnerDoc(), how can doc be a descendant of popup->GetContent()? It would be an ancestor, surely?

Comment 15

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

Attachment: patch

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

What about comment #14?

Comment 17

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

ah, right. But ownerdoc check is still way faster than
nsContentUtils::ContentIsCrossDocDescendantOf. And that ownerdoc check should
be a common case.

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 421326 [details] [diff] [review]
patch

r+ if you remove the doc == popup->GetContent()->GetOwnerDoc() check. I don't think it's worth making a tiny optimization to the performance of dispatching mouse events to popups.

Comment 19

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

Attachment: patch

Comment 20

[Myk Melez [:myk] [@mykmelez]]

In my tests using the testcase, the patch works great!  I don't experience the problem with key events (f.e. tabbing from element to element and typing into an input field), which seem to work fine even if I don't manually focus the iframe content.

Comment 21

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 421329 [details] [diff] [review]
patch

This would go on the 1.9.2 branch very easily. Probably too late to respin now, but 1.9.2.1 for sure.

Comment 22

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

http://hg.mozilla.org/mozilla-central/rev/942185a89154

I'm not sure why this should go to 1.9.2.x.

Comment 23

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I thought this was desperately wanted for 3.7.

Comment 24

[Mark Finkle (:mfinkle) (use needinfo?)]

I have pointed several extension developers at this bug recently. browser / iframe in a panel seems to be a common UI pattern.

Comment 25

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

Interesting. It is still very unclear to me what is the good use case for having
iframe in xul:panel.
Though, perhaps isolating jetpacks in iframe or something like that.

Comment 26

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #25)
> Though, perhaps isolating jetpacks in iframe or something like that.

That's exactly the use case.

Panel <https://wiki.mozilla.org/Labs/Jetpack/JEP/23> (bug 494238) is one of the highest priority features for Jetpack.  And Jetpack principles include extensions not having chrome privileges and extensions being able to implement their UX using web technologies (especially HTML).

Our strategy for Jetpack Panel has thus been to implement it using content iframes inside XUL panels, since that seems to be the best way to accomplish our goals.  And since the feature is one of Jetpack's highest priorities, we want it as soon as possible, i.e. in Firefox 3.6.

So it is eminently sensible to make every effort to get this Jetpack panel blocker on the 1.9.2 branch.

Comment 27

[Henri Sivonen (:hsivonen)]

Any ideas why the test case for this bug fails with the HTML5 parser? (Bug 539896)

Comment 28

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 421329 [details] [diff] [review]
patch

a=beltzner for 1.9.2.2

Comment 29

[Myk Melez [:myk] [@mykmelez]]

Landed on 1.9.2 branch:
  http://hg.mozilla.org/releases/mozilla-1.9.2/rev/a57bc2f49c9d

Comment 30

[gg0687644]

The fix is not on on the current xulrunner 1.9.2 release version (1.9.2.3705), which can be found at http://releases.mozilla.org/pub/mozilla.org/xulrunner/releases/1.9.2/.

However, version 1.9.2.3743 shipped with Firefox 3.6.3 has fixed the bug.

I thought the xulrunner release closely matches that of Firefox, so why is this delay?