Page MenuHomePhabricator

FOUC of horizontal rule in Vector sidebar
Closed, ResolvedPublic

Description

The horizontal rule that is specified as the background for div.portal elements is flickering briefly on page load:

Event Timeline

ori created this task.Feb 14 2015, 8:26 AM
ori raised the priority of this task from to High.
ori updated the task description. (Show Details)
ori added projects: Vector, Performance Issue.
ori added a subscriber: ori.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 14 2015, 8:26 AM
ori set Security to None.Feb 14 2015, 8:28 AM
ori added subscribers: Krinkle, matmarex.
gerritbot added a subscriber: gerritbot.

Change 190645 had a related patch set uploaded (by Bartosz Dziewoński):
Don't use JavaScript to style first sidebar portlet

https://gerrit.wikimedia.org/r/190645

Patch-For-Review

In the off-chance some user script or style actually depends on .first, I'd feel safer if the skin (vector.php) just adds the class statically.

Or the JS adding .first could be kept as is. This would remove the FOUC while ensuring that any user script/style still works, if that is a concern.

Also, if we want to still give some support to non grade A browsers, then the CSS for .first could be kept as is too.

I think what Bartosz's main concern is, why does that class have to be added with JavaScript, which may incur a rendering delay?

ori added a comment.Feb 14 2015, 10:15 PM

In the off-chance some user script or style actually depends on .first, I'd feel safer if the skin (vector.php) just adds the class statically.

I searched both the MediaWiki: and User: namespaces and found only one case: ru:User:Зелёный Кошак/edittools.js. So I think we can get rid of it and let this particular user know via a friendly note on his or her talk page. Thoughts?

@Edokter, I meant that maybe we should ensure support for the first class
(by adding it in JS) while still adding in the new sibling selector change.
This would probably take care of the FOUC as well as support user
styles/scripts.
Edokter added a comment.

I think what Bartosz's main concern is, why does that class have to be
added with JavaScript, which may incur a rendering delay?

TASK DETAIL

https://phabricator.wikimedia.org/T89542

REPLY HANDLER ACTIONS

Reply to comment or attach files, or !close, !claim, !unsubscribe or

!assign <username>.

EMAIL PREFERENCES

https://phabricator.wikimedia.org/settings/panel/emailpreferences/

To: matmarex, Edokter
Cc: polybuildr, Edokter, gerritbot, matmarex, Krinkle, ori, Aklapper,
GWicke, Jdforrester-WMF, Jay8g

Change 190645 merged by jenkins-bot:
Don't use JavaScript to style first sidebar portlet

https://gerrit.wikimedia.org/r/190645

Jdforrester-WMF closed this task as Resolved.Feb 15 2015, 3:40 AM
Jdforrester-WMF added a subscriber: Jdforrester-WMF.

Fixed?

Change 190699 had a related patch set uploaded (by Ori.livneh):
Don't use JavaScript to style first sidebar portlet

https://gerrit.wikimedia.org/r/190699

Patch-For-Review

Change 190699 merged by jenkins-bot:
Don't use JavaScript to style first sidebar portlet

https://gerrit.wikimedia.org/r/190699

Change 190700 had a related patch set uploaded (by Ori.livneh):
Don't use JavaScript to style first sidebar portlet

https://gerrit.wikimedia.org/r/190700

Patch-For-Review

Change 190700 merged by jenkins-bot:
Don't use JavaScript to style first sidebar portlet

https://gerrit.wikimedia.org/r/190700

I know we don't support IE6 anymore, but does that mean we should actively break IE6?

What was wrong with my suggestion of simply adding the class statically, thus maintaining compatibility (and skinability I might add), instead of using a 'modern' CSS selector, which otherwise adds no benifit at all?

Edokter reopened this task as Open.Feb 15 2015, 9:17 AM

You know what... I am annoyed enough to reopen this.

matmarex removed matmarex as the assignee of this task.Feb 15 2015, 1:07 PM

We're not breaking IE 6, it was already not working.

I did the patch the way I did it because it was simpler and neater; I made the effort to support IE 7+. I didn't think that rewriting the PHP code to do this, only to support IE 6 better, would be a good use of my time. (It's not like this a critical feature, like reading or editing pages; it's a little grey line in the sidebar…) If you think it's a good use of yours, please submit a patch and I'll be happy to merge it.

Note, though, that Vector already looks quite messed up on IE 6 (although it is perfectly functional).

matmarex lowered the priority of this task from High to Low.Feb 15 2015, 1:08 PM

Is there an issue with not removing the CSS off .first and also leaving in the JS that adds in .first? The FOUC will be handled by using the sibling selector and IE6 will not break because the JS will still be working.

Edokter closed this task as Resolved.Feb 15 2015, 1:41 PM
Edokter claimed this task.

@polybuildr, That would result in more redundant code.

@matmarex, Just leave it... I'm just annoyed it got to this messed up state because people insist on using new methods, even when trusted methods work just as well, without any extra work, and which would keep the site presentable for older browsers.

I think that's just terribly lazy...

@polybuildr, there should be no issues with doing that, but it won't help
IE 6, because MediaWiki doesn't send any JavaScript to it (it's
blacklisted in /resources/src/startup.js along with a few other outdated
or intentionally limited browsers).