1515265 - [remote-dbg-next] The context menu on dom inspector of tab which opened from about:debugging differs from normal dom inspector

Status: VERIFIED FIXED

(DevTools :: about:debugging, defect, P1)

Description

[Daisuke Akatsuka (:daisuke)]

STRs:
1. Open new about:debugging
2. Click "Inspect" button of any tabs
3. Show context menu on the dom inspector

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.

Comment 1

[Daisuke Akatsuka (:daisuke)]

Attachment: context-menu-from-aboutdebugging.png

This is a screenshot for the context menu of inspector which opened from about:debugging.

Comment 2

[Daisuke Akatsuka (:daisuke)]

Attachment: context-menu-on-normal-inspector.png

And normal inspector.

Comment 3

[Julian Descottes [:jdescottes]]

Note that some context menus are fine:
- inspector rule-view
- console message list
- netmonitor request list
- storage main panel

Some weird ones:
- debugger: has the right content but seem to be "resized", menus look odd (can also make the UI overflow)
- console input: there is a second context menu appearing behind the main one, tiny and empty, but still clickable?

Comment 4

[Julian Descottes [:jdescottes]]

It seems that the issue only occurs when we attach events on iframe elements. If fixing the underlying issue is too complicated, let's simply do this.

Comment 5

[Julian Descottes [:jdescottes]]

Attachment: fix_markup_view_contextmenu.diff

Here is the fix for the markup view for instance.

Comment 6

[Daisuke Akatsuka (:daisuke)]

This will be fixed after resolving bug 1519332.

Comment 7

[Daisuke Akatsuka (:daisuke)]

Since the platform won't fix the event propagation, we fix this from devtools side.
I'll try with Julian's approach from now.

Comment 8

[Daisuke Akatsuka (:daisuke)]

I have investigated the difference of both normal devtools and devtools-toolbox.
https://docs.google.com/spreadsheets/d/15p6gZ77QBaVvQzS8srHVXCAjZ3NqU7UT28olWnCYPqo/edit?usp=sharing

I will proceed with following approach.
Firstly, make context menu of devtools to show correctly. From result of investigation, the targets are MarkupView, ComputedView in Inspector and Storage. And I'd like to discuss about the rest of points such as show/hide the contentAreaContextMenu.

Comment 9

[Daisuke Akatsuka (:daisuke)]

Attachment: Bug 1515265: Move a place where add contextmenu listener to show the context menu in markup view. r=jdescottes!

Comment 10

[Daisuke Akatsuka (:daisuke)]

Attachment: Bug 1515265: Call stopPropagation() and preventDefault() at all places where handle contextmenu event and show the popup. r=jdescottes!

Depends on D17457

Comment 11

[Daisuke Akatsuka (:daisuke)]

Attachment: Bug 1515265: Hide the context menu on about:devtools-toolbox when target was not text input field. r=jdescottes!

Depends on D17458

Comment 12

[Daisuke Akatsuka (:daisuke)]

Attachment: Bug 1515265: Hide unnecessary menuitems from the context menu. r=jdescottes!

Depends on D17459

Comment 13

[Daisuke Akatsuka (:daisuke)]

Attachment: Bug 1515265: Move a logic which avoids to inspect to ContextMenuChild. r=jdescottes!

To control the context menu from outside.

Depends on D17460

Comment 14

[Julian Descottes [:jdescottes]]

Thanks a lot for the patches so far Daisuke! As discussed I'd like us to get more feedback for the last 3 patches in the queue. I think we can split the bug. Can you file a followup to "Hide default/webcontent context menu items in about:devtools-toolbox" and move the 3 last patches there?

My only other comment would be: do you think we can add a mochitest to check that the context menu of the markup view is correct?

Comment 15

[Daisuke Akatsuka (:daisuke)]

Thanks, Julian!
I filed bug 1522400.
Also, yes, I'll add a mochitest for the context menu on markup view.

Comment 16

[Phabricator Automation]

Comment on attachment 9038739 [details]
Bug 1515265: Hide the context menu on about:devtools-toolbox when target was not text input field. r=jdescottes!

Revision D17459 was moved to bug 1522400. Setting attachment 9038739 [details] to obsolete.

Comment 17

[Phabricator Automation]

Comment on attachment 9038740 [details]
Bug 1515265: Hide unnecessary menuitems from the context menu. r=jdescottes!

Revision D17460 was moved to bug 1522400. Setting attachment 9038740 [details] to obsolete.

Comment 18

[Phabricator Automation]

Comment on attachment 9038741 [details]
Bug 1515265: Move a logic which avoids to inspect to ContextMenuChild. r=jdescottes!

Revision D17461 was moved to bug 1522400. Setting attachment 9038741 [details] to obsolete.

Comment 19

[Daisuke Akatsuka (:daisuke)]

Attachment: Bug 1515265: Add a test for context menu of the markup view in about:devtools-toolbox. r?jdescottes

Depends on D17458

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e51312da073f
Move a place where add contextmenu listener to show the context menu in markup view. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/635099091943
Call stopPropagation() and preventDefault() at all places where handle contextmenu event and show the popup. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/319ae34add89
Add a test for context menu of the markup view in about:devtools-toolbox. r=jdescottes

Comment 21

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/e51312da073f
https://hg.mozilla.org/mozilla-central/rev/635099091943
https://hg.mozilla.org/mozilla-central/rev/319ae34add89

Comment 22

[Liviu Seplecan]

I've verified this issue on latest Nightly 68.0a1, Firefox dev edition 67.0, release 66.0.1 => on all versions the issue won't occur.