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

RESOLVED FIXED in Firefox 14

Status

()

Firefox
Developer Tools: Inspector
P1
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: harth, Assigned: harth)

Tracking

unspecified
Firefox 14
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

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

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

5 years ago
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.

Updated

5 years ago
Depends on: 717916
(Assignee)

Comment 1

5 years ago
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.
Assignee: nobody → fayearthur
Attachment #603974 - Flags: review?(paul)
(Assignee)

Comment 2

5 years ago
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.
Attachment #603974 - Attachment is obsolete: true
Attachment #603975 - Flags: review?(paul)
Attachment #603974 - Flags: review?(paul)

Updated

5 years ago
Attachment #603975 - Attachment is patch: true

Comment 3

5 years ago
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+

Updated

5 years ago
Status: NEW → ASSIGNED

Comment 4

5 years ago
triage, filter on centaur
Priority: -- → P1
Whiteboard: [firefox14-wanted]

Comment 5

5 years ago
I realise we don't have any test for that. Can we get one before landing this?

Comment 6

5 years ago
@harth, can you do it, or do you want me to do it?
(Assignee)

Comment 7

5 years ago
Created attachment 615680 [details] [diff] [review]
patch unbitrotted plus test

New patch, updated and with a test.
Attachment #603975 - Attachment is obsolete: true
Attachment #615680 - Flags: review?(paul)
(Assignee)

Comment 8

5 years ago
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.
Attachment #615680 - Attachment is obsolete: true
Attachment #615686 - Flags: review?
Attachment #615680 - Flags: review?(paul)
(Assignee)

Updated

5 years ago
Attachment #615686 - Flags: review? → review?(paul)

Updated

5 years ago
Attachment #615686 - Flags: review?(paul) → review+

Updated

5 years ago
Whiteboard: [firefox14-wanted] → [land-in-fx-team]
(Assignee)

Comment 9

5 years ago
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
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 14
You need to log in before you can comment on or make changes to this bug.