Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

use DOMUtils's new pseudo-class lock API for locking pseudo-classes #4

Closed
wants to merge 1 commit into from

3 participants

Heather Arthur Steven Roussey Sebastian Zartner
Heather Arthur

This uses the new DOMUtils pseudo-class lock API from bug 708874 to toggle :hover, :active, and :focus on the CSS panel.

This is preferable to what Firebug currently has because setting these locks doesn't update the element's actual content state, rather it just updates the element's style. So mousing over another element doesn't lose the :hover, etc.

I can't figure out how to listen for when a new element is selected and listen for when Firebug is closed from the CSS panel. So right now the pseudo-class stays on the element until you manually uncheck it.

Steven Roussey
Owner

What version of Firebug has (or will have) this API?

It looks like we will need to keep a list of elements whose state we have changed, such that we can reset them on certain events. Obviously, resetting all of them on Firebug close is required. I can add some code to your patch to show how to listen to that particular event.

The other times to reset them seems to still be an open issue though. I often want to set the hover an element then select the child element to see how its CSS rules have changed. So resetting based on selecting another element doesn't seem like the solution. However, I'm not sure what is the solution if just clicking on the content page is not enough. Thoughts?

Heather Arthur

This API will be in Firefox 13.

Keeping a list would be hard. What Firefox devtools did is keep the lock around only if you move to an element within the hierarchy chain in the breadcrumbs. This allows inspecting parent elements with the lock after locking it on a child element.

You can see the feature in Aurora right now by right clicking the node's selector, instructions here: http://www.youtube.com/watch?feature=player_embedded&v=wuZB6JA4dCU.

Sebastian Zartner
Collaborator

As far as I saw the new functions are:

void addPseudoClassLock(in nsIDOMElement aElement, in DOMString aPseudoClass);
void removePseudoClassLock(in nsIDOMElement aElement, in DOMString aPseudoClass);
bool hasPseudoClassLock(in nsIDOMElement aElement, in DOMString aPseudoClass);
void clearPseudoClassLocks(in nsIDOMElement aElement);

So we obviously need to keep track of the elements with locked state to be able to call clearPseudoClassLocks() on them. Why do the functions not support a nsIDOMNodeList or why isn't there a function to clear the pseudo-class locks of all elements on the page?
We need to make sure the UX for this is clear. If the states should be reset by some events, it might be better to create a different UI than the Style side panel options menu items we have now. See also issue 3230.
Heather, do you know, if other pseudo-classes than :active, :hover and :focus are supported? I referring to your comment on bug 708874. See issue 4131.

Sebastian

Heather Arthur

It doesn't support NodeLists. You can iterate over a NodeList and use any of the functions on each node individually.

Check out the UI for this in the built-in Firefox devtools, I'd recommend playing around with it. It adds an orange ":hover" to the selectors in the breadcrumbs, for one.

Devtools removes the lock when you inspect a new node altogether, unless you are just navigating up the tree with the breadcrumbs, then the lock is retained. This covers all the most common use cases for wanting to keep a lock around.

All other pseudo-classes are supported, but the :visited issue is a whole different issue altogether. :visited rules aren't exposed to getComputedStyle() or getCSSStyleRules() due to a privacy change in Firefox, see bug 713106, and setting a pseudo-class lock isn't going to change that.

Sebastian Zartner
Collaborator

Check out the UI for this in the built-in Firefox devtools, I'd recommend playing around with it. It adds an orange ":hover" to
the selectors in the breadcrumbs, for one.

I did that and it's pretty hidden like in Firebug. I would like to make these options more obvious and move them away from the normal options, because they are not panel specific but element specific. At least they should be put into the context menu of the elements. That doesn't make it much more obvious but at least they would be right placed.
Setting a lock should also be indicated inside the HTML panel somehow, e.g. by changing the background color of the node or something like that.

All other pseudo-classes are supported

That's great. So maybe there should be an item for each one? Or do you think this would congest the UI?

but the :visited issue is a whole different issue altogether

Thanks for the hint.

Sebastian

Steven Roussey
Owner

I did that and it's pretty hidden like in Firebug

So hidden in fact, that I can figure out how to do it... :(

Heather Arthur

Devtools is planning on putting it in a special new node-specific menu on that "infobar" (the thing that show's the node's selector) soon to make it more discoverable. The HTML view could be a good place to put the control in Firebug, as a context menu on the elements of the tree.

I don't think you want an item for each pseudo-class in existence, and there are too many them. There's only really need for :hover, :active, and :focus because those are dynamic and hard to pin down when debugging.

If this is going to take a long time to figure out it might be worthwhile to just hook up the new API to the current UI at first, as it's still a significant improvement in UX.

Sebastian Zartner
Collaborator

Heather, would it be possible to help us here with an updated patch according to the things we discussed?

Sebastian

Heather Arthur

It might be a better use of time for someone more familiar with Firebug's code to do this. Were the functionality and UI specified?

Sebastian Zartner
Collaborator

Were the functionality and UI specified?

In my previous comment I tried to explain how I imagine it to work. We discussed this in a conf call earlier and it seemed people agreed to that UI.

Sebastian

Heather Arthur

I wouldn't be able to get around to it anytime soon, but I'm here to answer questions.

Sebastian Zartner
Collaborator

After hesitating for that long I decided to use your patch. Currently I applied it to a separate branch.
The reason I did so now is that the patch really has benefits over what we currently have. I put the UI changes into a new issue.

Sebastian

Sebastian Zartner SebastianZ closed this May 23, 2012
Jan Odvarko janodvarko referenced this pull request May 25, 2012
Closed

Issue 5159 #7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 1 unique commit by 1 author.

Mar 21, 2012
Heather Arthur use DOMUtils's new pseudo-class lock API for locking pseudo-classes f6dffa8
This page is out of date. Refresh to see the latest.
48  extension/content/firebug/css/cssElementPanel.js
@@ -492,33 +492,45 @@ CSSElementPanel.prototype = Obj.extend(CSSStyleSheetPanel.prototype,
492 492
 
493 493
         if (Dom.domUtils && this.selection)
494 494
         {
495  
-            var state = safeGetContentState(this.selection);
496 495
             var self = this;
497 496
 
498 497
             ret.push("-");
499 498
 
500 499
             ret.push(
501 500
                 {
  501
+                    label: "style.option.label.hover",
  502
+                    type: "checkbox",
  503
+                    checked: self.hasPseudoClassLock(":hover"),
  504
+                    tooltiptext: "style.option.tip.hover",
  505
+                    command: function()
  506
+                    {
  507
+                        self.togglePseudoClassLock(":hover");
  508
+                    }
  509
+                }
  510
+            );
  511
+
  512
+            ret.push(
  513
+                {
502 514
                     label: "style.option.label.active",
503 515
                     type: "checkbox",
504  
-                    checked: state & STATE_ACTIVE,
  516
+                    checked: self.hasPseudoClassLock(":active"),
505 517
                     tooltiptext: "style.option.tip.active",
506 518
                     command: function()
507 519
                     {
508  
-                        self.updateContentState(STATE_ACTIVE, !this.getAttribute("checked"));
  520
+                        self.togglePseudoClassLock(":active");
509 521
                     }
510 522
                 }
511 523
             );
512  
-
  524
+   
513 525
             ret.push(
514 526
                 {
515  
-                    label: "style.option.label.hover",
  527
+                    label: "style.option.label.focus",
516 528
                     type: "checkbox",
517  
-                    checked: state & STATE_HOVER,
518  
-                    tooltiptext: "style.option.tip.hover",
  529
+                    checked: self.hasPseudoClassLock(":focus"),
  530
+                    tooltiptext: "style.option.tip.focus",
519 531
                     command: function()
520 532
                     {
521  
-                        self.updateContentState(STATE_HOVER, !this.getAttribute("checked"));
  533
+                        self.togglePseudoClassLock(":focus");
522 534
                     }
523 535
                 }
524 536
             );
@@ -527,17 +539,29 @@ CSSElementPanel.prototype = Obj.extend(CSSStyleSheetPanel.prototype,
527 539
         return ret;
528 540
     },
529 541
 
530  
-    updateContentState: function(state, remove)
  542
+    hasPseudoClassLock: function(pseudoClass)
  543
+    {
  544
+        return Dom.domUtils.hasPseudoClassLock(this.selection, pseudoClass);
  545
+    },
  546
+
  547
+    togglePseudoClassLock: function(pseudoClass)
531 548
     {
532 549
         if (FBTrace.DBG_CSS)
533  
-            FBTrace.sysout("css.updateContentState; state: " + state + ", remove: " + remove);
  550
+            FBTrace.sysout("css.togglePseudoClassLock; pseudo-class: " + pseudoClass);
534 551
 
535  
-        Dom.domUtils.setContentState(remove ? this.selection.ownerDocument.documentElement :
536  
-            this.selection, state);
  552
+        if (Dom.domUtils.hasPseudoClassLock(this.selection, pseudoClass))
  553
+            Dom.domUtils.removePseudoClassLock(this.selection, pseudoClass);
  554
+        else
  555
+            Dom.domUtils.addPseudoClassLock(this.selection, pseudoClass);
537 556
 
538 557
         this.refresh();
539 558
     },
540 559
 
  560
+    clearPseudoClassLocks: function()
  561
+    {
  562
+        Dom.domUtils.clearPseudoClassLocks(this.selection);
  563
+    },
  564
+
541 565
     addStateChangeHandlers: function(el)
542 566
     {
543 567
         this.removeStateChangeHandlers();
7  extension/locale/en-US/firebug.properties
@@ -494,6 +494,13 @@ style.option.tip.active=Display :active pseudo-class styles
494 494
 style.option.label.hover=:hover
495 495
 style.option.tip.hover=Display :hover pseudo-class styles
496 496
 
  497
+# LOCALIZATION NOTE (style.option.tip.focus):
  498
+# Style side panel option tooltip (located in tab's option menu). If the option is set to true,
  499
+# the Style side panel will simulate the element being focused and therefore display
  500
+# :focus pseudo-class styles
  501
+style.option.label.focus=:focus
  502
+style.option.tip.focus=Display :focus pseudo-class styles
  503
+
497 504
 # LOCALIZATION NOTE (css.option.Expand_Shorthand_Properties, css.option.tip.Expand_Shorthand_Properties):
498 505
 # CSS panel/Style side panel option (located in tab's option menu). If set to true,
499 506
 # CSS shorthand properties like 'margin' will be split into their components, e.g. 'margin-top',
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.