contextmenu event of about:* page of xul which use iframe and also open on tab does not send to the parents correctly

VERIFIED INVALID

Status

()

defect
VERIFIED INVALID
6 months ago
4 months ago

People

(Reporter: daisuke, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Posted patch trace.patchSplinter Review

STRs:

  1. Open about:debugging-new
  2. Click "Inspect" button of one of tabs
    (Open devtools in new tab)
  3. Show context menu on the dom inspector of the tab

ER:
The context menu should be same to normal dom inspector.

AR:
The context is not for the inspector. For example, context menu of normal inspector has "Create new Node" item though, the inspector which opened from about:debugging page does not have.
Please see the attachment of the original bug 1515265.

The reason seems what the context menu event does not send to the parents correctly.
(I have taken various advices from Masayuki during investigating! Thank you very much!)
I had been investigating how the event walks to the parents with the attached patch. When show the context menu of normal inspector which can be opened by Tools -> Web Developer -> Inspector, traces like below from event source to the root.

span 0x143e75b00
div 0x143e75800
li 0x143e75780
ul 0x143e7f680
div 0x143e14c00
div 0x143e14780
body 0x143e14080
html 0x13d7e1980
#document 0x1372f7000
0x143e45000
iframe 0x13e334f30
div 0x13602aa00
div 0x135fb9e00
div 0x13d738880
div 0x13d738980
div 0x13d739280
div 0x13618f880
div 0x135fb9800
body 0x1316d9280
html 0x134927180
#document 0x128f57000
0x140128800
iframe 0x12a47f6a0
vbox 0x13d717790
box 0x13d55cc10
vbox 0x13d55c550
vbox 0x13d543dc0
window 0x13d513e50
#document 0x111bfa000
0x11001b800
iframe 0x1284499c0
vbox 0x13d7fa820
hbox 0x13d7fa8b0
tabpanels 0x11018a280
tabbox 0x11018a1f0
vbox 0x11018a160
hbox 0x10f251d30
deck 0x1101808b0
window 0x110c99ee0
#document 0x10a20f000

The other hand, when show the context menu of inspector of new tab, traces like below.

span 0x143ea3280
div 0x143ea3100
li 0x143ea3080
ul 0x14324e980
div 0x14321d100
div 0x14321d000
body 0x14321c900
html 0x142153700
#document 0x142a24000
0x105388c00
0x139b04300
browser 0x129fa2c00
stack 0x13d7fa790
vbox 0x13d7fa820
hbox 0x13d7fa8b0
tabpanels 0x11018a280
tabbox 0x11018a1f0
vbox 0x11018a160
hbox 0x10f251d30
deck 0x1101808b0
window 0x110c99ee0
#document 0x10a20f000

When we compare, the latter we can know that the event sends to <browser> element from #document via two EventTarget[1] that does not have node name, then parents which is between #document and <browser> was shortcutted. I invesigated though, the two EventTarget was nsGlobalWindowInner[2] and InProcessTabChildMessageManger[3]. And since this InProcessTabChildMessageManger points <browser> as next parent, the event was shortcutted.
I investigated why got such the behavior though, mParentTarget which sets in nsGlobalWindowInner::UpdateParentTarget[4] was same instance of InProcessTabChildMessageManger which has <browser> as the parent for all of nsGlobalWindowInner instances.
So, after through the 1st nsGlobalWindowInner, the parent will be same, then go to <browser>.

I'm not familiar with this related technology though, regardless all of xul of about:* page is running on main process only, use the message manager. This might be cause makes weird behavior?

[1] https://searchfox.org/mozilla-central/source/dom/events/EventTarget.h#49
[2] https://searchfox.org/mozilla-central/source/dom/base/nsGlobalWindowInner.h
[3] https://searchfox.org/mozilla-central/source/dom/base/InProcessTabChildMessageManager.h
[4] https://searchfox.org/mozilla-central/source/dom/base/nsGlobalWindowInner.cpp#1850

Hello Ollie,
Masayuki introduced you for this bug, couldn’t you take a look?

Flags: needinfo?(bugs)

https://searchfox.org/mozilla-central/source/dom/events/nsIEventListenerService.idl#82 can be useful here to debug the issue.

So this is about contextmenu in devtools? This smells like a devtools issue. Does it create different kinds of documents when debugging parent process chrome privileged documents?
The propagation depends on what "ChromeEventHandler" points to.

But I don't quite understand the event paths. Are they for the opened pages or for devtools.
I'd expect devtools UI to be the same, but those event paths look different.
The latter has
window 0x13d513e50 which hints there is xul:window in there...

Events propagate differently in browser chrome
https://searchfox.org/mozilla-central/rev/c21d6620d384dfb13ede6054015da05a6353b899/dom/base/nsFrameLoader.cpp#1930-1932
So events get through all the ancestor documents by default. One can change that behavior with type attribute on iframe/browser element.

Flags: needinfo?(bugs)

Ah, I may be wrong. In this case, the simplest testcase is like this:

- <hbox>
  - <browser>
    - #document (HTML)
      - html
        + head
        - body
           - iframe
             - #document (HTML)
               - html

When mouse event is fired in the leaf document, it won't be propagated from/to its parent HTML document. You can check this with setting <iframe srcdoc="<div id='foo'>here</div>"></iframe> into the <textarea> in https://d-toybox.com/studio/lib/pointing_device_event_viewer.html and click it. Then, the red-dashed <div> element in the parent document never receives mousedown/mouseup/click events.

Perhaps, now I understand this issue correctly but I'd like smaug to check whether my thought is correct or not.

If <iframe> elements (meaning HTML's <iframe> elements, not XUL's) are in a XUL <browser> element, any events fired in their contentDocuments are never fired on parent documents in the <browser> element. This must be standardized behavior.

On the other hand, if HTML <iframe>s are just in XUL <iframe> elements (i.e., no parent <browser> elements), events in HTML <iframe> elements are also fired on parent documents too.

Currently, devtools depend on the latter behavior because they add event listeners to <iframe> elements to catch right-click events in the sub-document. Therefore, when they are loaded into a <browser> element with new UI, they failed to catch "contextmenu" events.

So, I think that this should be marked as INVA.

Flags: needinfo?(bugs)

Yeah, even propagation has been different in chrome vs content code forever, and changing it would most probably break other parts of Firefox UI.
This very much sound like something to be handled in devtools

Please reopen if needed.

Status: NEW → RESOLVED
Closed: 6 months ago
Flags: needinfo?(bugs)
Resolution: --- → INVALID

Thank you! And sorry for you getting involved.

Status: RESOLVED → VERIFIED

Nah, this is a good reminder that we should actually document event propagation somewhere. I guess EventDispatcher.h would be a good place.

Olli, Masayuki, thank you very much!
So, I'll try to fix from devtools side, but please let me ask to you again if I face issue about event related.
Thanks!

(In reply to Olli Pettay [:smaug] from comment #7)

Nah, this is a good reminder that we should actually document event propagation somewhere. I guess EventDispatcher.h would be a good place.

+1 :)

Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.