Closed Bug 733612 Opened 8 years ago Closed 8 years ago

Add a pseudo-class lock menu to the infobar node menu

Categories

(DevTools :: Inspector, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 14

People

(Reporter: harth, Assigned: harth)

References

Details

(Whiteboard: [fixed-in-fx-team])

Attachments

(1 file, 3 obsolete files)

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.
Depends on: 717916
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.
Assignee: nobody → fayearthur
Attachment #603974 - Flags: review?(paul)
Sorry, here's the real patch.
Attachment #603974 - Attachment is obsolete: true
Attachment #603975 - Flags: review?(paul)
Attachment #603974 - Flags: review?(paul)
Attachment #603975 - Attachment is patch: true
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
Attachment #603975 - Flags: review?(paul) → review+
Status: NEW → ASSIGNED
triage, filter on centaur
Priority: -- → P1
Whiteboard: [firefox14-wanted]
I realise we don't have any test for that. Can we get one before landing this?
@harth, can you do it, or do you want me to do it?
Attached patch patch unbitrotted plus test (obsolete) — Splinter Review
New patch, updated and with a test.
Attachment #603975 - Attachment is obsolete: true
Attachment #615680 - Flags: review?(paul)
As discussed on irc, we want to reposition the infobar when the pseudo-class is added to the selector. This new patch includes this.
Attachment #615680 - Attachment is obsolete: true
Attachment #615686 - Flags: review?
Attachment #615680 - Flags: review?(paul)
Attachment #615686 - Flags: review? → review?(paul)
Attachment #615686 - Flags: review?(paul) → review+
Whiteboard: [firefox14-wanted] → [land-in-fx-team]
http://hg.mozilla.org/integration/fx-team/rev/0ba7e290a08f
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/0ba7e290a08f
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 14
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.