Closed Bug 931990 Opened 11 years ago Closed 11 years ago

[markup view] Context menu disappearing in markup tree during after mutation

Categories

(DevTools :: Inspector, defect)

Product:
DevTools
Component:
Inspector
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
Firefox 28

People

(Reporter: bgrins, Assigned: bgrins)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

context-menu.patch
(deleted), patch
paul
: review+
Details | Diff | Splinter Review 
I've noticed that the context menu tends to disappear sometimes in the markup view.  I have tracked this down to a reproducible case, and it appears to have something to do with mutations in a node that has been selected.

STR:

Inspect a node that has DOM changes being applied.
Right click on any node in the markup view.
A context menu opens (with options like Edit as HTML, Copy Outer HTML, etc).

Expected:
Context menu remains open until user dismisses it.

Actual:
Context menu closes after a mutation occurs.


Below some sample markup that causes the problem (can see this same markup in action at http://fiddle.jshell.net/bgrins/uDHDY/show/).  In this case, the problem does not start until you inspect the <h1> tag (which has a <span> tag as a child that is changing).

<html>
<head>
  <title>Changes</title>
  <script>
    var current = 1;
    var interval = setInterval(function() {
      document.querySelector("h1 span").textContent = current++;
    }, 1000);
  </script>
</head>

<body>
  <h1>Number of seconds since loading: <span /></h1>
  <p>
    Context menu disappears every second.
  </p>
</body>
</html>
Blocks: DevToolsPaperCuts
This line seems to be triggering the issue: https://mxr.mozilla.org/mozilla-central/source/browser/devtools/markupview/markup-view.js#1047:  this._viewbox.setAttribute("style", ...).

Since the preview panel is preffed off by default, we can fix this issue for most people by just returning early out of this function if it isn't enabled.
(In reply to Brian Grinstead [:bgrins] from comment #1)
> This line seems to be triggering the issue:
> https://mxr.mozilla.org/mozilla-central/source/browser/devtools/markupview/
> markup-view.js#1047:  this._viewbox.setAttribute("style", ...).
> 
> Since the preview panel is preffed off by default, we can fix this issue for
> most people by just returning early out of this function if it isn't enabled.

Actually, this is not what is causing the issue - need to keep looking into it.
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Attached patch context-menu.patch (deleted) — — Splinter Review 
Paul,
Asking you for review since this affects the breadcrumb stuff.  A few things here:

* Main thing in this patch: only call hideNodeMenu if update is being called *not* from a markupmutation.  If the markup mutation is a delete on the selected node, the menu will be hidden anyway, as update will be called with the new-node-front.
* Somewhat unrelated but discovered when tracking this down - don't run logic for the preview panel in markup view if it is preffed off.
* Remove unused `invalidateHierarchy` function from breadcrumbs.
Attachment #828805 - Flags: review?(paul)
Attachment #828805 - Flags: review?(paul) → review+
Whiteboard: fix → [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/05b04015a884
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 28
Keywords: verifyme
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0		
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:28.0) Gecko/20100101 Firefox/28.0		
Mozilla/5.0 (X11; Linux i686; rv:28.0) Gecko/20100101 Firefox/28.0		

Reproduced the initial issue on a nightly from 2013-10-28, verified as fixed on latest Aurora 28.0a2.
Status: RESOLVED → VERIFIED
Keywords: verifyme
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: