[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

bzimport set Reference to bz46401.
kaldari created this task.Via LegacyMar 21 2013, 3:36 AM
Krinkle added a comment.Via ConduitMar 21 2013, 11:22 PM

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.

Krinkle added a comment.Via ConduitMar 26 2013, 5:02 PM

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

Krinkle added a comment.Via ConduitMar 26 2013, 5:57 PM

(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.

RobLa-WMF added a comment.Via ConduitMar 27 2013, 1:27 AM

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

greg added a comment.Via ConduitMar 28 2013, 3:50 PM

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.

kaldari added a comment.Via ConduitMar 28 2013, 5:33 PM
  • Bug 46367 has been marked as a duplicate of this bug. ***
kaldari added a comment.Via ConduitMar 28 2013, 5:41 PM

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).

kaldari added a comment.Via ConduitMar 28 2013, 10:22 PM

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

Krinkle added a comment.Via ConduitMar 29 2013, 4:44 PM

(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.

Krinkle added a comment.Via ConduitMar 29 2013, 8:21 PM

Change-Id: I7b1562b12c8ed1a0286c19ef9db8f76870d4f69e

Aklapper added a comment.Via ConduitApr 8 2013, 2:40 PM

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

Jdforrester-WMF added a comment.Via ConduitApr 8 2013, 4:53 PM

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

MarkAHershberger added a comment.Via ConduitApr 8 2013, 5:26 PM

Should this fix go into the 1.19 branch?

gerritbot added a comment.Via ConduitApr 8 2013, 5:26 PM

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

hashar added a comment.Via ConduitApr 8 2013, 7:21 PM

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).

gerritbot added a comment.Via ConduitApr 8 2013, 7:23 PM

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

Catrope added a comment.Via ConduitApr 8 2013, 7:25 PM

(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.

gerritbot added a comment.Via ConduitApr 8 2013, 8:43 PM

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

Unknown Object (User) added a comment.Via ConduitApr 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',
...

kaldari added a comment.Via ConduitApr 8 2013, 11:50 PM

@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.Via ConduitApr 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.

Krinkle added a comment.Via ConduitApr 9 2013, 1:58 AM

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, ..

kaldari added a comment.Via ConduitApr 9 2013, 2:26 AM

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).

Krinkle added a comment.Via ConduitApr 10 2013, 6:52 AM

(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.

Krinkle added a comment.Via ConduitApr 10 2013, 6:53 AM

Ib54a3e78710 has been backported to REL1_21.

kaldari added a comment.Via ConduitApr 10 2013, 5:51 PM

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

kaldari added a comment.Via ConduitApr 10 2013, 5:57 PM

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).

Nikerabbit added a comment.Via ConduitApr 11 2013, 1:16 PM

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?

RobLa-WMF added a comment.Via ConduitApr 29 2013, 7:05 PM

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.

Aklapper added a comment.Via ConduitApr 30 2013, 10:40 AM

(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.

Anomie added a comment.Via ConduitApr 30 2013, 6:07 PM

(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.

Nikerabbit added a comment.Via ConduitMay 2 2013, 5:47 AM

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:

Nemo_bis added a comment.Via ConduitMay 2 2013, 5:53 AM

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:

Nemo_bis added a comment.Via ConduitMay 2 2013, 6:07 AM

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.

Aklapper added a comment.Via ConduitMay 2 2013, 2:11 PM

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 .

Add Comment