Page MenuHomePhabricator

Remove the extra animations happening before a page becomes ready
Open, Stalled, LowPublicBUG REPORT

Assigned To
None
Authored By
Ebrahim
Oct 6 2024, 7:57 AM
Referenced Files
F57620177: a.webm
Oct 16 2024, 7:59 PM
F57620120: a.webm
Oct 16 2024, 7:19 PM
F57606101: a.webm
Oct 11 2024, 12:07 PM
F57593033: A.webm
Oct 6 2024, 8:36 PM
F57591908: B.webm
Oct 6 2024, 7:57 AM
F57591888: A.webm
Oct 6 2024, 7:57 AM

Description

This issue is about multiple bugs I believe *probably* will eventually need one generalization of the approach of https://codesearch.wmcloud.org/search/?q=vector-animations-ready&files=&excludeFiles=&repos= (applied to <html>) and https://codesearch.wmcloud.org/search/?q=minerva-animations-ready&files=&excludeFiles=&repos= (applied to <body> element)

The first bug:

Steps to replicate the issue:

(perhaps this is Safari related? if it doesn't happen in your browser please test Safari or Epiphany of Linux)

What happens?:
Search box glitches from light color to dark each time sometimes

What should have happened instead?:
There is a

.cdx-text-input__input:enabled {
  ...
  transition-property: background-color,color,border-color,box-shadow;
  ...
}

Which should be turned into something like

.vector-animations-ready,
.minerva-animations-ready {
  .cdx-text-input__input:enabled {
    ...
    transition-property: background-color,color,border-color,box-shadow;
    ...
  }
}

But that class is theme specific and not possible to use with Codex

The second bug:

Steps to replicate the issue:

  • Clone tip of tree of CategoryTree
  • Add this to a page
<categorytree mode="pages">C</categorytree>

<div dir="rtl" class="mw-content-rtl">
<categorytree mode="pages">C</categorytree>
</div>

<div dir="ltr" class="mw-content-ltr">
<categorytree mode="pages">C</categorytree>
</div>
  • Refresh the page each time

What happens?:
Some of the arrows animates each time by refreshing the page by Ctrl+R / Command+R

What should have happened instead?:
That to not happen as the fix by https://gerrit.wikimedia.org/r/c/mediawiki/extensions/CategoryTree/+/1078048 using

.vector-animations-ready,
.minerva-animations-ready {
	.CategoryTreeToggle {
		transition: transform 250ms ease;
	}
}

See also similar issues that found or fixed on later comments

Event Timeline

Change #1078048 had a related patch set uploaded (by Ebrahim; author: Ebrahim):

[mediawiki/extensions/CategoryTree@master] Try to avoid initial animation of arrow

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

The triangle arrow on the enhanced recent changes list (Special:RecentChanges?enhanced=1) is also affected.

A replace of

transition: transform 250ms ease;

by

.vector-animations-ready & {
        transition: transform 250ms ease;
}

in https://gerrit.wikimedia.org/g/mediawiki/core/+/a4a53d00aca3e52db6a2a5dbc737eda9e47d2dce/resources/src/mediawiki.special.changeslist.enhanced.less#59 avoids the rotation animation on loading.

Change #1078106 had a related patch set uploaded (by Ebrahim; author: Ebrahim):

[mediawiki/skins/Vector@master] Disable initial animation of watch star

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

There is also one in watch animation which is in Vector skin code so can use Vector's own style right now, https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/1078106

Change #1078106 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Disable initial animation of watch star

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

Ebrahim renamed this task from Generalize the idea of vector-animations-ready and minerva-animations-ready to Remove extra animations before page load.Oct 7 2024, 1:58 PM
Ebrahim updated the task description. (Show Details)
Ebrahim renamed this task from Remove extra animations before page load to Remove the extra animations happening before a page becomes ready.Oct 7 2024, 2:01 PM
Ebrahim updated the task description. (Show Details)

I've suggested a tag since all phab tickets should have an associated project.

I actually think it would be good to replace minerva-animations-ready and vector-animations-ready with a generic skin-animations-ready and move this code into core (resources/src/mediawiki.page.ready/ready.js) so skins like Timeless, Monobook etc can benefit from this change.

I've suggested a tag since all phab tickets should have an associated project.

I actually think it would be good to replace minerva-animations-ready and vector-animations-ready with a generic skin-animations-ready and move this code into core (resources/src/mediawiki.page.ready/ready.js) so skins like Timeless, Monobook etc can benefit from this change.

A generic class would be a good idea since there are already multiple skins using it already. It would be helpful for extensions and gadgets as well.

The name of this generic class is a discussion point. Maybe there will be other use-cases than animations with the property transition. The class gets added after DOMContentLoaded. An adequate name for the class is content-loaded.

Watch star on mobile interface still has an animation, filed as T376872

Change #1079150 had a related patch set uploaded (by Ebrahim; author: Ebrahim):

[mediawiki/skins/MinervaNeue@master] Remove initial animation of watch star

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

Change #1079150 merged by jenkins-bot:

[mediawiki/skins/MinervaNeue@master] Remove initial animation of watch star

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

Ebrahim removed Jdlrobson as the assignee of this task.

This isn't solved though, we still need a generic class to be implemented, one that Codex team also agrees with, so we can fix all the similar issues in CategoryTree, Codex and the different skins. Someone from Codex team can have a look at the suggestion of Fomafix? Thanks

Change #1079454 had a related patch set uploaded (by Ebrahim; author: Ebrahim):

[mediawiki/core@master] Add content-loaded class to <body>

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

Based on your comments I uploaded a patch, it follows Minerva on the way it's applied rather than Vector (applied to <body> rather than <html> and simple use of $ instead of manual use of DOMContentLoaded) as the Vector one was almost unused before the works here and Minerva one is older it seems.

The only thing is, is this a convention Codex also can accept? Maybe Codex itself should apply this or maybe Codex should let its non-MediaWiki clients to use such convention or maybe Codex should apply this itself regardless also?

Change #1079463 had a related patch set uploaded (by Ebrahim; author: Ebrahim):

[design/codex@main] WIP: Enable input's transitions after content ready

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

Test wiki on Patch demo by Ebrahim using patch(es) linked to this task was deleted:

http://patchdemo.wmcloud.org/wikis/af1c064d50/w/

Change #1079479 had a related patch set uploaded (by Fomafix; author: Fomafix):

[mediawiki/core@master] Enhanced changeslist: Activate animation after content loaded

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

WIP fix can be seen in https://patchdemo.wmcloud.org/wikis/fc32db7fae

First login to the wiki then enable the dark mode then open Special:Random https://patchdemo.wmcloud.org/wikis/fc32db7fae/wiki/Special:Random multiple times with browser cache disabled and look at the search box, the text input isn't turning from light to dark (unlike what can be seen on the first screencast here) but the button is, which should be fixable also if we find the right convention.

So to recap from what I see we need a convention like this https://gerrit.wikimedia.org/r/1079454 somewhere so that something like this https://gerrit.wikimedia.org/r/1079463 can happen, unless, we want some other solutions for it unlike what Minerva and Vector does right now, some of such solutions can be seen at https://stackoverflow.com/q/14389566 and https://stackoverflow.com/q/27938900

(Sorry for CC spam, I just wanted to make sure someone able to make such a convention and decision is CCed, I try to keep the noise low from now on)

Change #1079488 had a related patch set uploaded (by Ebrahim; author: Ebrahim):

[mediawiki/core@master] Disable all animations and transitions before page is ready

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

Actually reviewing stackoverflow again I thought this can have much less painful alternative solution like this also https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1079488

Under what circumstances does this bug happen? It seems I can't reproduce it. I suspect there might be a gadget or browser extension involved.

When you server-render, or client-inject an HTML element with class X on the page, and class X has a CSS property and an animation for that same property, it doesn't animate. The web standards are clear on this. Browsers don't do that. It only animates if the element already exists for at least one whole render cycle with a different value for the same property. Might that be happening here? Is something adding these styles too late? Or is something modifying one of the elements in an unsupported way?

  1. https://www.mediawiki.org/w/index.php?title=Project:Sandbox&action=submit
  2. Paste the below
  3. Show preview.
<categorytree mode="pages">C</categorytree>

<div dir="rtl" class="mw-content-rtl">
<categorytree mode="pages">C</categorytree>
</div>

<div dir="ltr" class="mw-content-ltr">
<categorytree mode="pages">C</categorytree>
</div>
  • Vector. Doesn't happen. It renders correclty initially without animation, exactly as the CSS standards says the browser should. Animations only happen from an already-existing state to a different one. Never for the initially rendered state. This works both for server-rendered and client-injected elements.
  • Vector 22. Doesn't happen.
  • Vector 22 in dark mode, then click "Show preview" to reload. Doesn't happen. I thought it might happen here, because if dark mode is being applied by lazy JavaScript, that would create a second state. Although that second state would be limited to colors, not transform rotation. In any event, no rotation took place here. Also, client preferences (such as dark mode) we to implement in ResourceLoader such that no reflow happens. These classes are set in the HTML head before the body starts precisely so that there is only 1 initial state.

@Fomafix as you first noticed this issue, can you please try to fill this communication gap also?

I haven't introduced the aforementioned classes to two different skins myself so some browser extension or gadget from my side being a culprit of something that has already a workaround in the code is a bit unlikely. I don't know about the standard's position on this, I just have seen that two classes being available for the purpose on two different skins and some people having the same issue on stackoverflow also. https://stackoverflow.com/q/14389566

In order to make the reproduction reliable please disable your browser cache, at least that makes it reliable for me both locally and non-locally,

On https://www.mediawiki.org/wiki/User:Ebrahim/categorytree when "Disable cache" from Safari's dev console is enabled:

Locally reproducing both issues (search box going from light to dark and arrows) using Firefox and it's 'Disable Cache' option,

And this isn't limited to 'Disable Cache' option, it happens for me every time for the night mode search box without any dev console configuration.

I just checked this happens only in Firefox and Safari but not in Chrome (sorry for not checking that earlier) so have filed bug against the two also,

So maybe instead of fixing it here maybe we can wait browsers to fix this.

I can't reproduce this, not locally, or in prod. Not in Safari or in Firefox.

I note that dark mode isn't available in Vector by default locally. I added $wgVectorNightMode['logged_in'] = true; in LocalSettings to make this available. Is that how you did it?

I see a ULS widget in the video as well. Can you reproduce it without any LocalSettings changes or extensions loaded? (besides wfLoadSkin vector and wgVectorNightMode, that is).

I can't reproduce this, not locally, or in prod. Not in Safari or in Firefox.

I was seeing the search box issue going from light to dark for a while, it was annoying but I was ignoring it till Fomafix noted something similar after my changes on CategoryTree, then I went to find the root of these, so at least I can say I'm not alone on seeing that.

Is that how you did it?

I was puzzled with that weeks ago I wanted to fix dark mode related issues but then learnt I can just install beta features extension and after that dark mode can be accessed when "Accessibility for Reading (Vector 2022)" from Beta Features of Preferences when Vector 2022 is enabled locally.

I see a ULS widget in the video as well. Can you reproduce it without any LocalSettings changes or extensions loaded? (besides wfLoadSkin vector and wgVectorNightMode, that is).

It happens even only when these are enabled,

$wgDefaultSkin = 'vector-2022';
wfLoadSkin( 'Vector' );
wfLoadExtension( 'BetaFeatures' );
wfLoadExtension( 'CategoryTree' );

Now that it's fixed in Chrome, to me this is a browser bug and not that worth to fix. (and sorry again for CC spam I made, I thought it may need a new cross project (MediaWiki to Codex) convention, if we still want to fix this ourselves, it may still do)

Change #1079454 abandoned by Ebrahim:

[mediawiki/core@master] Add content-loaded class to <body>

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

Change #1079488 abandoned by Ebrahim:

[mediawiki/core@master] Disable all animations and transitions before page is ready

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

Change #1078048 abandoned by Ebrahim:

[mediawiki/extensions/CategoryTree@master] Try to avoid initial animation of arrow

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

I can reproduce the initial animation of the triangle on Special:RecentChanges?enhanced=1 even with disabled JavaScript and without any extensions and skins.

Even if it's a browser bug, since it's showing up in firefox and safari, I honestly think it's worth bypassing until the browser bug is fixed.

Jdrewniak changed the task status from Open to Stalled.Thu, Oct 31, 5:16 PM
Jdrewniak triaged this task as Low priority.
Jdrewniak subscribed.

Marking as stalled since it's not yet clear whether this is a browser bug or something related to our code.