Last Comment Bug 733612 - Add a pseudo-class lock menu to the infobar node menu
: Add a pseudo-class lock menu to the infobar node menu
Status: RESOLVED FIXED
[fixed-in-fx-team]
:
Product: Firefox
Classification: Client Software
Component: Developer Tools: Inspector (show other bugs)
: unspecified
: All All
: P1 normal (vote)
: Firefox 14
Assigned To: Heather Arthur [:harth]
:
:
Mentors:
Depends on: 717916
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-06 16:52 PST by Heather Arthur [:harth]
Modified: 2012-04-17 10:51 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Add pseudo-class lock menu to node menu and remove from context menu (116 bytes, patch)
2012-03-07 22:27 PST, Heather Arthur [:harth]
no flags Details | Diff | Splinter Review
Add pseudo-class lock menu to node menu and remove from context menu (3.00 KB, patch)
2012-03-07 22:31 PST, Heather Arthur [:harth]
paul: review+
Details | Diff | Splinter Review
patch unbitrotted plus test (5.64 KB, patch)
2012-04-17 05:58 PDT, Heather Arthur [:harth]
no flags Details | Diff | Splinter Review
new patch plus moving infobar (5.80 KB, patch)
2012-04-17 06:18 PDT, Heather Arthur [:harth]
paul: review+
Details | Diff | Splinter Review

Description Heather Arthur [:harth] 2012-03-06 16:52:45 PST
We need to add the pseudo-class lock menuitems to the infobar node menu from bug 717916. They are currently in a context menu, added in bug 707740.
Comment 1 Heather Arthur [:harth] 2012-03-07 22:27:59 PST
Created attachment 603974 [details] [diff] [review]
Add pseudo-class lock menu to node menu and remove from context menu

This patch adds the pseudo-class lock menu items to the infobar's node menu. It also removes them from the infobar's context menu.
Comment 2 Heather Arthur [:harth] 2012-03-07 22:31:08 PST
Created attachment 603975 [details] [diff] [review]
Add pseudo-class lock menu to node menu and remove from context menu

Sorry, here's the real patch.
Comment 3 Paul Rouget [:paul] 2012-03-08 09:44:05 PST
Comment on attachment 603975 [details] [diff] [review]
Add pseudo-class lock menu to node menu and remove from context menu

Review of attachment 603975 [details] [diff] [review]:
-----------------------------------------------------------------

r=+ with the new className.

::: browser/devtools/highlighter/highlighter.jsm
@@ +500,5 @@
> +    let separator = this.chromeDoc.createElement("menuseparator");
> +    menu.appendChild(separator);
> +
> +    menu.addEventListener("popupshowing", function() {
> +      let items = menu.getElementsByClassName("pseudo-class-menuitem");

I usually prefer menu.querySelectorAll(".pseudo-class-menuitem")
(Up to you)

@@ +557,4 @@
>        let item = this.chromeDoc.createElement("menuitem");
>        item.setAttribute("type", "checkbox");
>        item.setAttribute("label", pseudo);
> +      item.className = "pseudo-class-menuitem";

Can you prefix the class with: "highlighter-"?
Like: highlighter-pseudo-class-menuitem
Comment 4 Paul Rouget [:paul] 2012-03-14 09:15:10 PDT
triage, filter on centaur
Comment 5 Paul Rouget [:paul] 2012-04-04 11:44:30 PDT
I realise we don't have any test for that. Can we get one before landing this?
Comment 6 Paul Rouget [:paul] 2012-04-05 12:29:16 PDT
@harth, can you do it, or do you want me to do it?
Comment 7 Heather Arthur [:harth] 2012-04-17 05:58:15 PDT
Created attachment 615680 [details] [diff] [review]
patch unbitrotted plus test

New patch, updated and with a test.
Comment 8 Heather Arthur [:harth] 2012-04-17 06:18:56 PDT
Created attachment 615686 [details] [diff] [review]
new patch plus moving infobar

As discussed on irc, we want to reposition the infobar when the pseudo-class is added to the selector. This new patch includes this.
Comment 9 Heather Arthur [:harth] 2012-04-17 06:38:52 PDT
http://hg.mozilla.org/integration/fx-team/rev/0ba7e290a08f
Comment 10 Rob Campbell [:rc] (:robcee) 2012-04-17 10:51:15 PDT
https://hg.mozilla.org/mozilla-central/rev/0ba7e290a08f

Note You need to log in before you can comment on or make changes to this bug.