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

VERIFIED FIXED in Firefox 66

Status

defect
P1
normal
VERIFIED FIXED
6 months ago
3 months ago

People

(Reporter: daisuke, Assigned: daisuke)

Tracking

(Blocks 1 bug)

Trunk
Firefox 66
Dependency tree / graph

Firefox Tracking Flags

(firefox66 verified, firefox67 verified, firefox68 verified)

Details

Attachments

(6 attachments, 3 obsolete attachments)

526.70 KB, image/png
Details
762.89 KB, image/png
Details
2.30 KB, patch
Details | Diff | Splinter Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
Assignee

Description

6 months ago
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.
Assignee

Comment 1

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

Comment 2

6 months ago
And normal inspector.
Assignee

Updated

6 months ago
Summary: The context menu on dom inspector of tab differs from normal dom inspector → [remote-dbg-next] The context menu on dom inspector of tab which opened from about:debugging differs from normal dom inspector
Priority: -- → P2
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?
Assignee

Updated

5 months ago
Assignee: nobody → dakatsuka
Status: NEW → ASSIGNED
Priority: P2 → P1

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.

Here is the fix for the markup view for instance.

Assignee

Updated

5 months ago
Depends on: 1519332
Assignee

Comment 6

5 months ago

This will be fixed after resolving bug 1519332.

Assignee

Comment 7

5 months ago

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

Assignee

Comment 8

5 months ago

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.

Assignee

Comment 13

5 months ago

To control the context menu from outside.

Depends on D17460

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?

Flags: needinfo?(dakatsuka)
Assignee

Updated

5 months ago
See Also: → 1522400
Assignee

Comment 15

5 months ago

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

Flags: needinfo?(dakatsuka)

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.

Attachment #9038739 - Attachment is obsolete: true

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.

Attachment #9038740 - Attachment is obsolete: true

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.

Attachment #9038741 - Attachment is obsolete: true

Comment 20

5 months ago
Pushed by dakatsuka@mozilla.com:
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

5 months ago
bugherder
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 66

Comment 22

3 months ago

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.

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.