Page MenuHomePhabricator

[Regression] mw.loader: Code should execute after styles being loaded (wait for async cssText buffering)
Closed, ResolvedPublic

Description

After doing some debugging, it looks like change I430fba99 is causing several different rendering problems in PageTriage:

  1. In Firefox, a race condition apparently caused by JS executing before CSS is applied (bug 46367)
  2. In Chrome, a strange rendering issue regarding the placement of the sort buttons (although the CSS seems to actually be correct)
  3. In Safari, rendering issues with both the placement of the filter flyout and the sort buttons
  4. In all browsers, a flash of unstyled content (which is strange since the change was apparently to address unwanted repainting)

You can see all these issues at http://test2.wikipedia.org/wiki/Special:NewPagesFeed (but not http://test2.wikipedia.org/wiki/Special:NewPagesFeed?debug=true).

I was able to work around the first problem with change Ibcbef930, but haven't had time to solve the other problems. (We're in the home stretch of getting Echo ready for deployment to en.wiki.) Unless change I430fba99 is going to be reverted, I imagine this will split into separate bugs once the problems are diagnosed.


Version: 1.21.x
Severity: normal

Details

Reference
bz46401

Event Timeline

bzimport raised the priority of this task from to High.Nov 22 2014, 1:35 AM
bzimport set Reference to bz46401.

Before cssText buffer became async this was fine. In fact, there are various guards to make sure this happens in the same order. This is by design.

When we made it async this was done at a low level and escaped the guards at the higher level. We need to fix this by keeping the state of the cssText buffer the same way we do for the code when executing multiple closures.

Wait for all asyncs of cssText buffer to have finished before scheduling the asyncs for the closure.

For the record, the error reported in those reports is not the regression reported in this bug.

(In reply to comment #2)

I think this all may be related to the issues we're seeing reported on both
commons and enwiki Village Pumps (and probably elsewhere);

http://commons.wikimedia.org/wiki/Commons:
FORUM#komprimiertes_JavaScript_kaputt.3F
https://commons.wikimedia.org/wiki/Commons:
Village_pump#Strange_problem_with_CSS
https://en.wikipedia.org/wiki/Wikipedia:
Village_pump_%28technical%29#CSS_errors_with_search_box.2C_etc..3F
https://en.wikipedia.org/wiki/Wikipedia:
Village_pump_%28technical%29#Errors_with_editing_interface_and_sidebar

Timo, could you look into this?

See bug 46575.

Thanks Krinkle for the quick fix on bug 46575. I'm bumping this one down to high priority.

Krinkle/Kaldari: Do you think the issue described in bug 27698 comment 9 is the same as this?

If so, this bug's priority might need to be increased again.

  • Bug 46367 has been marked as a duplicate of this bug. ***

The timing suggests it's likely a race condition related to this bug (it was reported 3 days after change I430fba99 was deployed to en.wiki). I've also marked bug 46367 as a duplicate (which already has a workaround in place).

What would be the implications of reverting change I430fba99 in the meantime?

(In reply to comment #6)

Krinkle/Kaldari: Do you think the issue described in bug 27698 comment 9 is
the same as this?

Unlikely, I've seen that bug happen since for ever. Replied there, bug 27698 comment 15.

(In reply to comment #9)

What would be the implications of reverting change I430fba99 in the meantime?

I'll add a fix for this soon, but as long as there are no immediate problems, I'dl like to avoid going back and forth.

Change-Id: I7b1562b12c8ed1a0286c19ef9db8f76870d4f69e

Note to everybody following this:
Krinkle's patch in Gerrit is still awaiting more reviews.

Timo's code is now merged. Marking as resolved.

Should this fix go into the 1.19 branch?

Related URL: https://gerrit.wikimedia.org/r/58105 (Gerrit Change I7b1562b12c8ed1a0286c19ef9db8f76870d4f69e)

I have reverted the change with https://gerrit.wikimedia.org/r/#/c/58131/ because it caused an issue with JSDuck which eventually broke tests in the master branch (was bug 47018).

Related URL: https://gerrit.wikimedia.org/r/58134 (Gerrit Change Ib54a3e78710b7f798c6730d3f540d3284001e2de)

(In reply to comment #17)

Related URL: https://gerrit.wikimedia.org/r/58134 (Gerrit Change
Ib54a3e78710b7f798c6730d3f540d3284001e2de)

This is a resubmission of https://gerrit.wikimedia.org/r/#/c/58134/ with a small amendment to hopefully appease jsduck.

Related URL: https://gerrit.wikimedia.org/r/58216 (Gerrit Change Ib54a3e78710b7f798c6730d3f540d3284001e2de)

Unknown Object (User) added a comment.Apr 8 2013, 10:44 PM

I don't know if this directly related but after the above change was apllied to our MW 1.22 environment, QUnit's test locally show some unexpected behaviour. Resource definitions that include jquery modules seems to be loaded asynchronously resulting in sliders or datepickers to be "jumping" the page.

It appears that by the time the JS object(slider, datepicker) is instantiated and ready for display the corresponding jquery CSS is still not ready/loaded.

I tried explicitly to wait until the resource is loaded but that will not change the behaviour of the jquery components (declared as dependency).

mw.loader.using( 'ext.srf.eventcalendar', function(){
calendar.init( context, container, data );
...
} )

'ext.srf.eventcalendar' => ...
'dependencies' => array (
'jquery.ui.core',
'jquery.ui.widget',
'jquery.ui.datepicker',
'jquery.ui.slider',
...

@mwjames: Sounds similar to what I was seeing. Changing the dependencies wouldn't work, so I had to resort to using a setTimeout on the JS. Hopefully https://gerrit.wikimedia.org/r/#/c/58134/ will solve it.

Unknown Object (User) added a comment.Apr 9 2013, 1:53 AM

(In reply to comment #21)

wouldn't work, so I had to resort to using a setTimeout on the JS. Hopefully
https://gerrit.wikimedia.org/r/#/c/58134/ will solve it.

I still find myself confronted with the same problem (cache was cleared etc.) even after Change-Id: Ib54a3e78710b7f798c6730d3f540d3284001e2de. It is clearly visible during a page &action=purge request.

Please provide a more actionable use case. How do I reproduce this behaviour you're seeing?

One or more of these: Which extension, where is it installed and/or which version should I install locally, what page to look at, what to click, browser version, ..

Yeah, looks like Change-Id: Ib54a3e78710b7f798c6730d3f540d3284001e2de doesn't solve the regression for me either. PageTriage still requires the setTimeout workaround to fix bug 46367 (which is a more specific duplicate of this bug).

(In reply to comment #14)

Should this fix go into the 1.19 branch?

No, the this is a recent regression. 1.19 is not affected.

(In reply to comment #16)

I have reverted the change with https://gerrit.wikimedia.org/r/#/c/58131/
because it caused an issue with JSDuck which eventually broke tests in the
master branch (was bug 47018).

(In reply to comment #18)

(In reply to comment #17)

Related URL: https://gerrit.wikimedia.org/r/58134 (Gerrit Change
Ib54a3e78710b7f798c6730d3f540d3284001e2de)

This is a resubmission of https://gerrit.wikimedia.org/r/#/c/58134/ with a
small amendment to hopefully appease jsduck.

Merged, re-closing this bug.

The matter raised in comment 20 - comment 24 will either be solved by this or is related but different bug.

Ib54a3e78710 has been backported to REL1_21.

Looks like all the other issues, besides bug 46367, are resolved now. Thanks.

Sorry, I should clarify: All the issues described in the initial bug description have been resolved by change Ib54a3e78710, with the exception of bug 46367 which has been solved with a timeout workaround (change Ibcbef930).

I'm still seeing flash of unstyled content, for example in http://dev.translatewiki.net/w/i.php?title=Special:MainPage

Is the fix not working of is the cause different?

The fix for this issue has been identified as a possible cause of bug 47457. Krinkle is the process of fixing now, but I'm not sure if the fix for that bug will cause a regression here. Also, Niklas, I'm not sure what the answer to your question is; dev.translatewiki.net isn't loading for me at the moment.

(In reply to comment #29)

I'm still seeing flash of unstyled content, for example in
http://dev.translatewiki.net/w/i.php?title=Special:MainPage

Cannot reproduce. Browser and version info welcome.

(In reply to comment #31)

(In reply to comment #29)

I'm still seeing flash of unstyled content, for example in
http://dev.translatewiki.net/w/i.php?title=Special:MainPage

Cannot reproduce. Browser and version info welcome.

I can't reproduce either; the only thing I see when repeatedly refreshing the page in various browsers is that the images used in the page are sometimes slightly slow to load.

It seems very unlikely that whatever you're seeing is due to the major issues reported in this bug. If you can find a way to reliably reproduce your issue so others can see it, feel free to open a new bug report.

Created attachment 12234
Screenshot of unstyled content

I just rebooted and had problems reproducing this on the main page. I still see it on Special:Translate reliably.

Attached:

diagonally1.png (900×1 px, 98 KB)

Created attachment 12235
Special:Translate on Meta

(In reply to comment #34)

Created attachment 12234 [details]
Screenshot of unstyled content

I just rebooted and had problems reproducing this on the main page. I still
see
it on Special:Translate reliably.

I don't see that on the URL you linked, but I always see it on Meta lately. (I thought it just took some time to load.)

Attached:

TranslateUnstyled.png (197×887 px, 13 KB)

Also personal tools on mediawiki.org, at any page load: after the (0) is shown and before the grey/red Echo counter appears, the personal tools jump left-down and then go back in place.

I'm closing this again due to comment 33:

It seems very unlikely that whatever you're seeing is due to the major issues
reported in this bug. If you can find a way to reliably reproduce your issue
so others can see it, feel free to open a new bug report.

Please file a new ticket about the issue with meta and http://dev.translatewiki.net/wiki/Special:Translate?action=translate&group=!additions&filter=!translated .