Closed Bug 1486717 Opened 7 years ago Closed 6 years ago

Clicking "Inspect element" in responsive mode doesn't select the inspected element

Categories

(DevTools :: Responsive Design Mode, defect, P1)

Product:
DevTools
Component:
Responsive Design Mode
Platform:
Desktop
macOS
Type:
defect

Tracking

(firefox-esr60 unaffected, firefox-esr68 unaffected, firefox68 unaffected, firefox69 verified, firefox70 verified)

Status:
VERIFIED FIXED
Milestone:
Firefox 70
Tracking Flags:
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox68 --- unaffected
firefox69 --- verified
firefox70 --- verified

People

(Reporter: bbouvier, Assigned: jdescottes)

References

(Regression)

Details

(Keywords: regression, Whiteboard: [dt-q])

Attachments

(2 files)

Bug 1486717 - Do not emit contextmenu from parentprocess if target is the RDM iframe
47 bytes, text/x-phabricator-request
Details | Review
bug1486717.beta.patch
5.55 KB, patch
jdescottes
: review+
RyanVM
: approval-mozilla-beta+
Details | Diff | Splinter Review
STR: - go to demo.kresus.org - open browser devtools - select responsive design mode - right-click any element and select "inspect element (Q)" Expected: The inspector is opened, the DOM hierarchy opens up to the selected element, and information about the element shows up (CSS rules that apply, etc.). Observed: Only the <body> line is selected, the DOM hierarchy doesn't open, no elements seem to be selected (so no CSS information appears). Leaving the responsive design mode makes the element appear, though.
For what it's worth, doing the same STR than Comment 0, on `data:text/html,<meta charset=utf8><div>Inspect me</div>`, _does_ select the div in the inspector.
Important: in the STR from Comment 0, you have to open DevTools via a different panel than the inspector (e.g. Console). It looks like there's an initialization race.
Whiteboard: [dt-q]
@bbouvier I cannot reproduce this... is it now also working for you?
Flags: needinfo?(bbouvier)
Priority: -- → P3
Right, it seems it's been fixed by something. Thanks for checking in!
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: needinfo?(bbouvier)
Resolution: --- → WORKSFORME

Re-opening this since this issue has started happening again. Can replicate it using the steps described by the reporter.

Status: RESOLVED → REOPENED
OS: Unspecified → macOS
Hardware: Unspecified → Desktop
Resolution: WORKSFORME → ---

There seems to be a race in nsContextMenu.js. When the context menu opens, we execute

    this.targetSelectors = gContextMenuContentData
      ? gContextMenuContentData.popupNodeSelectors
      : [];

nsContextMenu.js#303-305
targetSelectors is what will be passed to DevTools to get the actual target for inspectElement. In most cases, when RDM is enabled gContextMenuContentData.popupNodeSelectors will be ".browser" which won't resolve to anything in the markup view. But if you wait before, you will get the correct selector.

Edit: Actually I can also see that when using RDM, we call twice nsContextMenu's openContextMenu. If you log the associated popupNodeSelectors, the first call will contain ".browser" and the second call will contain the proper selector.

I suspect a regression from Bug 1505909

Regressed by: 1505909

This has started consistently affecting users of Firefox 69 and 70. We should address this very soon. Thank you, Julian for investigating!

Priority: P3 → P1

Gabriel, can you help find an owner for this bug? It would be good to fix this for 69 if possible.

Flags: needinfo?(gl)

(In reply to Liz Henry (:lizzard) (use needinfo) from comment #12)

Gabriel, can you help find an owner for this bug? It would be good to fix this for 69 if possible.

Looks like Julian is already investigating.

Assignee: nobody → jdescottes
Flags: needinfo?(gl)
Attachment #9077340 - Attachment description: Bug 1486717 - Skip first context menu message in RDM → Bug 1486717 - Do not emit contextmenu from parentprocess if target is the RDM iframe

(In reply to Razvan Caliman [:rcaliman] from comment #11)

This has started consistently affecting users of Firefox 69 and 70. We should address this very soon. Thank you, Julian for investigating!

Sure thing! Good news is that the patch seems to apply cleanly on beta and also fixes the issue there, so uplifting should be easy once we get it in central

Status: REOPENED → ASSIGNED
Pushed by jdescottes@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/834110f6ca85 Do not emit contextmenu from parentprocess if target is the RDM iframe r=mconley,mtigley

https://hg.mozilla.org/mozilla-central/rev/834110f6ca85

Status: ASSIGNED → RESOLVED
Closed: 6 years ago → 6 years ago
status-firefox70: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 70

Please nominate this for Beta approval when you get a chance.

status-firefox-esr60: --- → unaffected
status-firefox-esr68: --- → unaffected
Flags: needinfo?(jdescottes)
Flags: in-testsuite+

Beta/Release Uplift Approval Request

  • User impact if declined: Right Click > Inspect Element no longer works when Responsive Design Mode is enabled
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: - open any website (eg https://www.mozilla.org/en-US/)
  • open DevTools
  • enable Responsive Design mode
  • right click on an element in the page, inside the RDM viewport
  • select "Inspect Element"

Expected Result: In the DevTools inspector, the Rules panel should show the rules applying to the element

Actual Result: The Rules panel shows "No element selected."

  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Simple javascript fix, covered by automated tests.
  • String changes made/needed:
Flags: needinfo?(jdescottes)
Attachment #9079260 - Flags: review+
Attachment #9079260 - Flags: approval-mozilla-beta?
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

Verified as fixed on latest Nightly 70.0a1(20190719094503)

status-firefox70: fixed → verified
Comment on attachment 9079260 [details] [diff] [review] bug1486717.beta.patch Fixes element inspection in RDM mode. Verified on Nightly and includes an automated test. Approved for 69.0b7.
Attachment #9079260 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Reproduced the issue on Beta 69.0b6(20190718172058). Verified as fixed on Beta 69.0b7(20190722201635) on Win10 x64, Ubuntu 18.04 and MacOS 10.14

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
status-firefox69: fixed → verified
Flags: qe-verify+

Thank you for fixing this! It was impeding my web development workflow. Cheers.

Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: