Closed Bug 533845 Opened 10 years ago Closed 10 years ago

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

Categories

(Core :: Web Painting, defect, P2)

defect

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- beta1+
status1.9.2 --- .2-fixed

People

(Reporter: jrmuizel, Assigned: smaug)

References

Details

Attachments

(2 files, 2 obsolete files)

I'm not sure exactly what's wrong but this was discussed in a meeting
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?
OS: Mac OS X → All
Hardware: x86 → All
A *minimal* testcase would be very useful!
Attached file 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.
Olli: have you made any progress figuring this out?
Sorry, no.
Assigning to myself so that I might remember/prioritize this.
Assignee: nobody → Olli.Pettay
Attached patch patch (obsolete) — Splinter Review
Myk, want to test this one?
I'm writing mochitest for this.
Note, the fix is for mouse events.
Though, key events seem to work usually, but not always.
Not sure what the problem with key events is. Something to do with focus.
Ok, if I manually focus iframe content (window.content.focus()) right after opening the panel, key events work just fine.
Ok, lets keep this bug for mouse event handling.
Summary: Content IFRAME in XUL panels don't work → Mouse events don't work in content iframe, if iframe is inside a xul panel
+              (!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.
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.
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?
Attached patch patch (obsolete) — Splinter Review
Attachment #421302 - Attachment is obsolete: true
Attachment #421326 - Flags: review?(roc)
ah, right. But ownerdoc check is still way faster than
nsContentUtils::ContentIsCrossDocDescendantOf. And that ownerdoc check should
be a common case.
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.
Attachment #421326 - Flags: review?(roc) → review+
Attached patch patchSplinter Review
Attachment #421326 - Attachment is obsolete: true
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 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.
Attachment #421329 - Flags: approval1.9.2.1?
http://hg.mozilla.org/mozilla-central/rev/942185a89154

I'm not sure why this should go to 1.9.2.x.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
I thought this was desperately wanted for 3.7.
I have pointed several extension developers at this bug recently. browser / iframe in a panel seems to be a common UI pattern.
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.
(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.
Any ideas why the test case for this bug fails with the HTML5 parser? (Bug 539896)
blocking2.0: ? → beta1
Priority: -- → P2
Comment on attachment 421329 [details] [diff] [review]
patch

a=beltzner for 1.9.2.2
Attachment #421329 - Flags: approval1.9.2.2? → approval1.9.2.2+
Whiteboard: [needs 192 landing]
Landed on 1.9.2 branch:
  http://hg.mozilla.org/releases/mozilla-1.9.2/rev/a57bc2f49c9d
Whiteboard: [needs 192 landing]
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?
Component: Layout: View Rendering → Layout: Web Painting
You need to log in before you can comment on or make changes to this bug.