stylo: site issue: visited links on BMO are not purple if you collapse and expand "Tracking"

VERIFIED FIXED in Firefox 56

Status

()

defect
P2
normal
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: darkspirit, Assigned: emilio)

Tracking

(Blocks 1 bug, {nightly-community})

Trunk
mozilla56
x86_64
All
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 verified)

Details

()

Attachments

(5 attachments, 2 obsolete attachments)

Nightly 56 x64 20170721100159 @ Debian Testing (Linux 4.11.0-1-amd64, Radeon RX480)

I can't reproduce bug 1380543 anymore, but this one is similar.

STR:
1. open https://bugzilla.mozilla.org/show_bug.cgi?id=1375906
2. collapse and expand "Tracking"
3. move your mouse over all blue (visited) "Depends on" links, they get purple
4. repeat (2), all links are blue again

See video.
Flags: needinfo?(jryans)
Has STR: --- → yes
If I zoom in/out or resize the window all blue (visited) links get purple (as they should have been).
FWIW, I can't reproduce this on today's Nightly on Mac OS X 10.12. Not sure if this is Linux specific...

Version: 	56.0a1
Build ID: 	20170721100207
OS:  	        Darwin 16.6.0
(In reply to Jeremy Chen [:jeremychen] UTC+8 from comment #2)
> FWIW, I can't reproduce this on today's Nightly on Mac OS X 10.12. Not sure
> if this is Linux specific...
> 
> Version: 	56.0a1
> Build ID: 	20170721100207
> OS:  	        Darwin 16.6.0

I have seen this several times over the last few days and I'm on Windows. I'll attach a screenshot of what I have seen. Unfortunately this issue isn't one I have been able to physically reproduce, it seems to happen sporadically and thus I have been unable to build a testcase for it.
This shows the display view from Mozilla inbound. As you can see some of the links for bug 1380133 are styled as visited and some are not.

As they are all the same link I would have expected them all to be styled the same.

Moving the cursor over the links leads to the styles updating.
OS: Linux → All
:emilio - Have you seen any bugs like this crop up in Stylo by any chance?
Flags: needinfo?(emilio+bugs)
Actually it seems that the issue is fully reproducible on todays nightly with the steps as outlined in comment #0.
Yes, this is fully reproducible...

I wasn't aware of this, but after taking a look I know what this is, and this is fallout for the huge rubberstamp that was bug 1367904, and yet another change that should've been done before that bug... :(
Assignee: nobody → emilio+bugs
Blocks: 1367904
Flags: needinfo?(emilio+bugs)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 10

2 years ago
mozreview-review
Comment on attachment 8889123 [details]
Bug 1383307: Use proper initializer in ServoStyleContext constructor.

https://reviewboard.mozilla.org/r/160156/#review165494
Attachment #8889123 - Flags: review+

Comment 11

2 years ago
mozreview-review
Comment on attachment 8889124 [details]
style: Fix relevant-link-visited logic in presence of nested links.

https://reviewboard.mozilla.org/r/160158/#review165496
Attachment #8889124 - Flags: review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
The second patch ("fix visited logic...") is what needs review, not the last one.
Flags: needinfo?(jryans)

Comment 18

2 years ago
mozreview-review
Comment on attachment 8889123 [details]
Bug 1383307: Use proper initializer in ServoStyleContext constructor.

https://reviewboard.mozilla.org/r/160156/#review165520
Attachment #8889123 - Flags: review+

Comment 19

2 years ago
mozreview-review
Comment on attachment 8889124 [details]
style: Fix relevant-link-visited logic in presence of nested links.

https://reviewboard.mozilla.org/r/160158/#review165522
Attachment #8889124 - Flags: review+

Comment 20

2 years ago
mozreview-review
Comment on attachment 8889161 [details]
Bug 1383307: Honor the relevant link visited pref.

https://reviewboard.mozilla.org/r/160260/#review165524

::: servo/components/style/context.rs:121
(Diff revision 1)
>  pub struct SharedStyleContext<'a> {
>      /// The CSS selector stylist.
>      pub stylist: &'a Stylist,
>  
> +    /// Whether visited styles are enabled.
> +    pub visited_styles_enabled: bool,

nit: leave a comment on when they won't be enabled
Attachment #8889161 - Flags: review?(manishearth) → review+

Comment 21

2 years ago
mozreview-review
Comment on attachment 8889162 [details]
stylo: Set the relevant link visited bit from servo.

https://reviewboard.mozilla.org/r/160262/#review165526
Attachment #8889162 - Flags: review?(manishearth) → review+

Comment 22

2 years ago
mozreview-review
Comment on attachment 8889163 [details]
Bug 1383307: Remove ServoStyleContext::UpdateWithElementState.

https://reviewboard.mozilla.org/r/160264/#review165528
Attachment #8889163 - Flags: review?(manishearth) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Assignee

Updated

2 years ago
Attachment #8889124 - Attachment is obsolete: true
Attachment #8889124 - Flags: review?(cam)
Assignee

Updated

2 years ago
Attachment #8889162 - Attachment is obsolete: true

Comment 26

2 years ago
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/e170c7eca2c4
Use proper initializer in ServoStyleContext constructor. r=manishearth
https://hg.mozilla.org/integration/autoland/rev/c92ef054df2b
Honor the relevant link visited pref. r=manishearth
https://hg.mozilla.org/integration/autoland/rev/469e3f8d6359
Remove ServoStyleContext::UpdateWithElementState. r=manishearth
BTW, I tried to build a test-case for this, but I found hard to write something that reliably failed before my patches because of the async link coloring stuff, which kinda sucks :(

Comment 28

2 years ago
mozreview-review
Comment on attachment 8889123 [details]
Bug 1383307: Use proper initializer in ServoStyleContext constructor.

https://reviewboard.mozilla.org/r/160156/#review165582

(Marking r+ for posterity, since mozreview didn't seem to want to forget about my review request after Manish reviewed.)
Attachment #8889123 - Flags: review?(cam) → review+
Priority: -- → P2
Verified fixed in Nightly 56 x64 20170725100346 @ Debian Testing (Linux 4.11.0-1-amd64, Radeon RX480).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.