Closed Bug 713106 Opened 9 years ago Closed 10 months ago

:visited styles not shown in Properties or Rule view

Categories

(DevTools :: Inspector: Rules, defect, P2)

defect

Tracking

(firefox71 fixed)

RESOLVED FIXED
Firefox 71
Tracking Status
firefox71 --- fixed

People

(Reporter: harth, Assigned: daisuke, Mentored)

References

(Depends on 1 open bug)

Details

(Keywords: dev-doc-needed, Whiteboard: [polish-backlog][difficulty=medium][firebug][lang=js][dt-q])

Attachments

(6 files, 3 obsolete files)

The Properties and Rules views don't show :visited styles if the selected element is a visited link.

Firebug has this problem too http://code.google.com/p/fbug/issues/detail?id=4131.
Most likely a result of the privacy changes to :visited style querying in bug 147777.

STR:
* Visit http://harthur.github.com/test-pages/pseudo-classes.html
* Inspect the first link named "visited website" (assuming you've visited google.com)
* Open the Style panel and the Rules view.

Expected result:
In the Rules view: see an "a:visited { color: hotpink; }" rule and a crossed-out, overridden "a:link { color: violet }"

In the Properties view: see "rgb(255, 105, 180)" for the "color" property

Actual result:
In the Rules view: see an un-overridden "a:link { color: violet }" rule, and no :visited rule.

In the Properties view: see "rgb(238, 130, 238)" for the "color" property
Yes, we should work out how to better handle pseudo-classes.
Whiteboard: [computedview][ruleview]
(this is not related to pseudo-classes)

triage, filter on centaur
Priority: -- → P3
Depends on: 729692, 729310
Duplicate of this bug: 762437
Surely the devtools can run with some extra privileges that would allow them to access the accurate visited information?
Priority: P3 → P2
Could we yet raise the priority of this bug?
It's also a problem for Firebug.

Honza
Whiteboard: [computedview][ruleview] → [computedview][ruleview][firebug]
I don't see them too on latest firefox nightly 33.0a1 (2014-06-24) and it makes me really sad. :(
Note to self:
Search for inspector.togglePseudoClass(':hover') and work from there.
Assignee: nobody → mratcliffe
Assignee: mratcliffe → m+bugzilla
Whiteboard: [computedview][ruleview][firebug] → [firebug][lang=js]
/browser/devtools/breadcrumbs.js:

```
const PSEUDO_CLASSES = [":hover", ":active", ":focus", ":visited"];
```

instead of

```
const PSEUDO_CLASSES = [":hover", ":active", ":focus"];
```

---

/browser/devtools/breadcrumbs.js:

add:

```
<menuitem id="node-menu-pseudo-visited"
        label=":visited" type="checkbox"
        oncommand="inspector.togglePseudoClass(':visited')"/>
```
Attached patch add-visited-styles-713106.patch (obsolete) — Splinter Review
(In reply to Erwann Mest from comment #8)
> /browser/devtools/breadcrumbs.js:
> 
> ```
> const PSEUDO_CLASSES = [":hover", ":active", ":focus", ":visited"];
> ```
> 
> instead of
> 
> ```
> const PSEUDO_CLASSES = [":hover", ":active", ":focus"];
> ```
> 
> ---
> 
> /browser/devtools/breadcrumbs.js:
> 
> add:
> 
> ```
> <menuitem id="node-menu-pseudo-visited"
>         label=":visited" type="checkbox"
>         oncommand="inspector.togglePseudoClass(':visited')"/>
> ```

I have created a patch for you. I have also done some stuff so that this works remotely and added a test.
Attachment #8447441 - Flags: review?(bgrinstead)
We didn't put :visited in because getCSSStyleRules and getComputedStyle won't return rules and properties for :visited nodes, which defeats the purpose. See the blocking bugs.
Yep, this bug is blocked by another one from platform.

Though, it must be fixed as it works perfectly in chrome and it's disturbing when you're developing a website.

@miker Sorry, I didn't do a patch because I mess my mercurial repo up. I will need to fix it. ;) (I only know git to be honest).

Thank you for your attention anyway.

@harth Do not hesitate to ping me when these bugs are fixed.
Comment on attachment 8447441 [details] [diff] [review]
add-visited-styles-713106.patch

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

The locking works well.  But as mentioned, until we can inspect the styles it doesn't really make sense to include it in the list.
Attachment #8447441 - Flags: review?(bgrinstead)
Yep, true. ;)
See Also: → 557577
Duplicate of this bug: 1062706
Has there been any updates here?
(In reply to :Ehsan Akhgari [Away: 1/29-2/20] (not reading bugmail, needinfo? me!) from comment #15)
> Has there been any updates here?

From what I understand, once Bugs 729310 and 729692 are resolved we can implement this in the toolbox.
Just got bitten by this. Any movement on the blockers for this bug?
(In reply to Potch [:potch] from comment #17)
> Just got bitten by this. Any movement on the blockers for this bug?

Bug 729310 is assigned (with an r+ed patch), but hasn't had activity for some months - I'll ask over there.  Bug 729692 hasn't had any activity yet.
Attachment #8447441 - Attachment is obsolete: true
Whiteboard: [firebug][lang=js] → [devedition-40][difficulty=medium][firebug][lang=js]
Whiteboard: [devedition-40][difficulty=medium][firebug][lang=js] → [polish-backlog][difficulty=medium][firebug][lang=js]
Duplicate of this bug: 945399
Whiteboard: [polish-backlog][difficulty=medium][firebug][lang=js] → [polish-backlog][difficulty=medium][firebug][lang=js][devtools-platform]
Whiteboard: [polish-backlog][difficulty=medium][firebug][lang=js][devtools-platform] → [polish-backlog][difficulty=medium][firebug][lang=js]
I think the rule view will also need to be updated in order to show invalid
visited properties to the user.  That is, for the CSS

a:visited {
  color: chartreuse;
  font-size: 23em;
}

... the font-size property should be struck through in the rule view, because
font-size is not on the approved list of properties for :visited
(see https://developer.mozilla.org/en-US/docs/Web/CSS/:visited)

It would be ideal if platform had a method to give out this information
rather than relying on a whitelist in the inspector.
Component: Developer Tools: Inspector → Developer Tools: CSS Rules Inspector
Duplicate of this bug: 1173505
See Also: → 1384099
I'd love to know, when or if this will be fixed.
I'm aware of security issues, but isn't is enough time passed for a solution to be made?
As an example only let Inspector get that information and not the scripts.
:visited is like a nemesis of web debugging.
Flags: needinfo?(m+bugzilla)
(Clearing assignee since I don't think this is in active work.)

We need to make some platform tweaks to expose the necessary info for DevTools and then we should be able to make progress here.
Assignee: m+bugzilla → nobody
Flags: needinfo?(m+bugzilla)
Product: Firefox → DevTools
Whiteboard: [polish-backlog][difficulty=medium][firebug][lang=js] → [polish-backlog][difficulty=medium][firebug][lang=js][dt-q]
Assignee: nobody → daisuke
Status: NEW → ASSIGNED

Depends on D45624

As discussed with Patrick on slack, we decided to split this bug into two.
In this bug, we will display :visited rule and lock/unlock for :visited pseudo-class.
And in the followup bug of this, we will support inactive CSS messages.

Blocks: 1581008

Comment on attachment 9092260 [details]
Bug 713106: Support inactive CSS messages. r?pbro!

Revision D45625 was moved to bug 1581008. Setting attachment 9092260 [details] to obsolete.

Attachment #9092260 - Attachment is obsolete: true

Comment on attachment 9092261 [details]
Bug 713106: Add an inactive CSS message for the color which has alpha channel. r?pbro!

Revision D45626 was moved to bug 1581008. Setting attachment 9092261 [details] to obsolete.

Attachment #9092261 - Attachment is obsolete: true

Depends on D45624

Depends on D45805

Depends on D45806

Pushed by dakatsuka.birchill@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1eb4d8d47675
Show :visited rule while the pseudo is locked. r=pbro,emilio
https://hg.mozilla.org/integration/autoland/rev/26c7c6044b01
Make :visited selector enabled to use from DevTools. r=pbro,emilio
https://hg.mozilla.org/integration/autoland/rev/205878310d59
Lock/unlock :visited pseudo-class. r=pbro
https://hg.mozilla.org/integration/autoland/rev/636ff6533a6f
Add test for InspectorUtils. r=emilio
https://hg.mozilla.org/integration/autoland/rev/512dabbc915d
Add test for visited element. r=pbro
Regressions: 1546369
Regressions: 1581582
Duplicate of this bug: 729310
QA Whiteboard: [qa-71b-p2]
You need to log in before you can comment on or make changes to this bug.