Closed
Bug 917863
Opened 11 years ago
Closed 11 years ago
Add XUL context menu back into rule and computed views
Categories
(DevTools :: Inspector, defect)
DevTools
Inspector
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 27
People
(Reporter: miker, Assigned: miker)
References
Details
Attachments
(1 file, 1 obsolete file)
28.32 KB,
patch
|
Details | Diff | Splinter Review |
The menu is to contain: - Select All - Copy As simple as it gets.
Assignee | ||
Comment 1•11 years ago
|
||
Attachment #806687 -
Flags: review?(jwalker)
Assignee | ||
Comment 2•11 years ago
|
||
Try: https://tbpl.mozilla.org/?tree=Try&rev=bd3b6beaf1e5
Comment 3•11 years ago
|
||
Comment on attachment 806687 [details] [diff] [review] xul-context-menu-917863.patch Review of attachment 806687 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/devtools/styleinspector/computed-view.js @@ +484,5 @@ > + _buildContextMenu: function() > + { > + let doc = this.styleDocument.defaultView.parent.document; > + > + let menu = this._contextmenu = this.styleDocument.createElementNS(XUL_NS, "menupopup"); Does this aliasing (menu/this._contextmenu) help readability significantly? I think there is a risk of confusing people with aliasing so we should only do it when there is a clear win. @@ +527,5 @@ > + */ > + _onContextMenu: function(event) { > + try { > + let win = this.styleDocument.defaultView; > + win.focus(); Do we need the 'win' variable? @@ +531,5 @@ > + win.focus(); > + > + this._contextmenu.openPopup( > + event.target.ownerDocument.documentElement, > + "overlap", event.clientX, event.clientY, true, false, null); Uber-nit: Could you use 4 space indents for continuation lines? A continuation line is very different from block indent, and particularly with if statements, using the same indent can be confusing.
Attachment #806687 -
Flags: review?(jwalker) → review+
Assignee | ||
Comment 4•11 years ago
|
||
Fixed leak: https://tbpl.mozilla.org/?tree=Try&rev=629343b39cda
Attachment #806687 -
Attachment is obsolete: true
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 6•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/59d5404eadf5
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 27
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•