Closed Bug 917863 Opened 7 years ago Closed 7 years ago

Add XUL context menu back into rule and computed views

Categories

(DevTools :: Inspector, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 27

People

(Reporter: miker, Assigned: miker)

References

Details

Attachments

(1 file, 1 obsolete file)

The menu is to contain:
 - Select All
 - Copy

As simple as it gets.
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+
https://hg.mozilla.org/integration/fx-team/rev/59d5404eadf5
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/59d5404eadf5
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 27
Depends on: 936047
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.