Closed Bug 1385831 Opened 2 years ago Closed 2 years ago

stylo: site issue: links are unclickable if you clicked on one on mcfit.com

Categories

(Core :: CSS Parsing and Computation, defect, P1)

x86_64
Linux
defect

Tracking

()

VERIFIED FIXED
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- wontfix
firefox56 --- wontfix
firefox57 --- fixed

People

(Reporter: darkspirit, Assigned: emilio)

References

(Blocks 1 open bug, )

Details

(Keywords: nightly-community)

Attachments

(3 files)

Attached video 2017-07-31_12-20-33.mp4
Nightly 56 x64 20170730100307 @ Debian Testing

STR (see video):
1. https://www.mcfit.com/de/
2a. If you have Basic Tracking Protection enabled, you can't click on any link/button
2b. Click on a link or button.
3. hover links/buttons, nothing happens, you can't click on them

As I haven't clicked on every link, I can't say "Visited links are unclickable" in the summary.
Has STR: --- → yes
Huh, interesting.

So you can't do anything because there's a huge <div> taking over the page.

That page is a bit |display: table| thing, so even though I'm not sure yet about what's going wrong, I think I have an idea...
Err, I meant that that <div> is a big display: table thing we probably failed to reframe properly.
Hmm... We're getting _so_ confused.

The page has some CSS that reads like:

.on-loading-complete .mod-load-indicator--mask{opacity:0;visibility:hidden;transition:300ms 500ms}.mod-flexbox{display:-webkit-flex;display:-ms-flexbox;display:flex;-webkit-flex-direction:row;-moz-flex-direction:row;-ms-flex-direction:row;flex-direction:row;-webkit-flex-wrap:wrap;-ms-flex-wrap:wrap;flex-wrap:wrap;margin-left:-10px;margin-right:-10px}

We're styling the |.on-loading-complete .mod-load-indicator--mask|, but somehow applying the .mod-flexbox rule... wtf.
Priority: -- → P1
(In reply to Emilio Cobos Álvarez [:emilio] from comment #2)
> Err, I meant that that <div> is a big display: table thing we probably
> failed to reframe properly.

Yeah, I can see the huge <div> with z-index: 999 blocking the links in the latest nightly Stylo as well.

Supposely the .on-loading-complete rule should be applied after finishing loading, and a { visibility: hidden; } should be applied to the huge <div> accordingly. I haven't managed to make a reduced testcase.... will keep working on making a reduced testcase tomorrow.
(In reply to Emilio Cobos Álvarez [:emilio] from comment #3)
> Hmm... We're getting _so_ confused.
> 
> The page has some CSS that reads like:
> 
> .on-loading-complete
> .mod-load-indicator--mask{opacity:0;visibility:hidden;transition:300ms
> 500ms}.mod-flexbox{display:-webkit-flex;display:-ms-flexbox;display:flex;-
> webkit-flex-direction:row;-moz-flex-direction:row;-ms-flex-direction:row;
> flex-direction:row;-webkit-flex-wrap:wrap;-ms-flex-wrap:wrap;flex-wrap:wrap;
> margin-left:-10px;margin-right:-10px}
> 
> We're styling the |.on-loading-complete .mod-load-indicator--mask|, but
> somehow applying the .mod-flexbox rule... wtf.

So the issue I was seeing here seems to be a devtools bug... The mod-load-indicator--mask does have display: table, and _does_ have visibility: hidden. So it sounds like we're missing a change-hint here or there, since reframing it does fix the issue.

I suspect this is again the missing restyling of table anon-boxes, but I'll try to reduce it as well.
Seems like they just add the on-loading-complete class to the body when the window onload event fires, so in theory it shouldn't be hard to make a reduced test-case... Let me try real quick.
Attached file reduced test-case.
I'm pretty sure this is because we're failing to restyle the anonymous cells that get created for the table. If I change the display to something that doesn't generate anonymous boxes (like display: block), I get the correct behavior back.
So this is instead because of the anonymous wrapper stuff. We need to get that fixed ASAP.

I'll talk with Boris in order to discuss what the best way to fix it is. I tried today one of the ways we discussed previously, but it's fairly non-trivial :-)
Depends on: 1340405, 1340404
Priority: P1 → --
Priority: -- → P1
Assignee: nobody → emilio+bugs
Duplicate of this bug: 1387857
No longer blocks: 1384602
Fixed by bug 1388625.
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Verified fixed in Nightly 57 x64 20170812100345 @ Debian Testing. Thank you!
Status: RESOLVED → VERIFIED
(This site is still unusable with Beta. Uplift? Or do you accept such bugs for Beta as long they are no crashes?)
(In reply to Jan Andre Ikenmeyer [:darkspirit] from comment #12)
> (This site is still unusable with Beta. Uplift? Or do you accept such bugs
> for Beta as long they are no crashes?)

Hm, tough. In general I think we're mostly just interested in stability metrics out of the beta experiment. On the other hand, if the lack of bug 1388625 is significant enough to make the site unusable, then we may want to uplift.

Cameron, given that bz is out, what do you think? Should we uplift bug 1388625? If so, can you nominate? Are there any other things that we'd need to uplift as well?
Flags: needinfo?(cam)
(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #13)
> Hm, tough. In general I think we're mostly just interested in stability
> metrics out of the beta experiment. On the other hand, if the lack of bug
> 1388625 is significant enough to make the site unusable, then we may want to
> uplift.

My general feeling is that if the beta experiment is really about stability metrics, and if mcfit.com isn't a major site and we haven't seen any other reports of sites being similarly unusable, that we shouldn't uplift it.  Also I just tried rebasing that bug's patches on beta, and I get a panic.  I tried with bug 1368290 included too, which I thought might be the other dependency, but it still panics.  I'm not sure it's worth putting in time to work out what's going on.
Flags: needinfo?(cam)
(In reply to Cameron McCormack (:heycam) from comment #14)
> if mcfit.com isn't a major site
It's the most popular fitness studio in Germany. But I haven't seen this bug elsewhere.
Poor german Beta users who thinks they test the next stable. xD

> I'm not sure it's worth putting in time to work out what's going on.
Fx 57 is more important then, I think.
(In reply to Jan Andre Ikenmeyer [:darkspirit] from comment #12)
> Or do you accept such bugs for Beta as long they are no crashes?
*don't you

(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #13)
> Hm, tough. In general I think we're mostly just interested in stability metrics out of the beta experiment.
Ok, thanks for clarifying!
Note that this bug also affected outlook mail... So there's probably a fair amount of people that would notice it...
Outlook Mail is pretty popular. live.com is Alexa rank #15. But given that the uplift panics and will require more debugging, I don't think we should spend any more time trying to uplift this fix.
You need to log in before you can comment on or make changes to this bug.