Page MenuHomePhabricator

Unify SVG optimization to Wikimedia coding conventions in MobileFrontend and Minerva
Closed, ResolvedPublic

Description

MF/MN's SVGO rules contain

- collapseGroups: false
- mergePaths: false

which seem to have been introduced in I7b09e89a5a72ff241c38914044b931520311efee and since them never been revisited on possible rendering issues.
The conventions in https://www.mediawiki.org/wiki/Manual:Coding_conventions/SVG#Exemplified_safe_configuration provided widely proofed harmless optimizations.

QA

  • Visit https://en.m.wikipedia.org/wiki/Front_end in older Grade A browsers like IE 11 and look out for icons being visually correct, not having rendering issues. Both as logged-in and logged-out user as some icons are login-state specific.

QA Results

ACStatusDetails
1T245296#5945501
2T245296#5945501

Event Timeline

Change 572305 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/skins/MinervaNeue@master] Unify SVGO rules to Wikimedia coding conventions and re-crush SVGs

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

Change 572317 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/extensions/MobileFrontend@master] Unify SVGO rules to Wikimedia coding conventions and re-crush SVGs

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

When above MN patch is merged, it would be helpful to have @Edtadros look over the changed icons once more.

Change 572317 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Unify SVGO rules to Wikimedia coding conventions and re-crush SVGs

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

Change 572305 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Unify SVGO rules to Wikimedia coding conventions and re-crush SVGs

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

Given it's assigned to Edward I'm guessing there is a request to QA it, but Edward does not QA task outside the sprint board.

Change 575811 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/skins/Vector@master] Add SVG minifier script npm run svgmin and unify SVGO rules to Wikimedia coding conventions

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

Change 575812 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/skins/Vector@master] Run SVG minifier in Vector skin

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

Test Result

Status: ❌ Fail
OS: Windows 7
Browser: IE11
Device: Browserstack

Test Artifact(s):
Visit https://en.m.wikipedia.org/wiki/Front_end in older Grade A browsers like IE 11 and look out for icons being visually correct, not having rendering issues. Both as logged-in and logged-out user as some icons are login-state specific.

❓ AC1: Logged In

Screen Shot 2020-03-05 at 9.02.36 AM.png (1×800 px, 280 KB)
Not icon related, but the letter g gets cut off.
Screen Shot 2020-03-05 at 9.03.35 AM.png (1×798 px, 292 KB)
Screen Shot 2020-03-05 at 9.02.16 AM.png (1×800 px, 206 KB)
Not icon related, but the letter g gets cut off.
Screen Shot 2020-03-05 at 9.15.07 AM.png (1×798 px, 278 KB)

❌ AC2: Not Logged In

Screen Shot 2020-03-05 at 8.52.50 AM.png (1×788 px, 323 KB)
Screen Shot 2020-03-05 at 8.55.40 AM.png (1×784 px, 174 KB)
Jdlrobson renamed this task from Unify SVG optimization to Wikimedia coding conventions to Unify SVG optimization to Wikimedia coding conventions in MobileFrontend and Minerva.Mar 5 2020, 6:24 PM

Both reported fails by @Edtadros are not connected with the SVG optimization, but separate issues. Will be filed into follow-up tasks.
Resolving this as 'successful' after moving it on kanban board.

Both reported fails by @Edtadros are not connected with the SVG optimization, but separate issues. Will be filed into follow-up tasks.
Resolving this as 'successful' after moving it on kanban board.

@Volker_E - sounds good. Resolving. Did you open a new task for the bug? If not, I can do it as well.

@Volker_E / @ovasileva here are the bug reports:

[Bug] Decenders get cut off in drop down menus T247145
[Bug] Edit Icons not the same size T247146

Let me know if you need more info or you'd like me to check different browsers/OSes.

TIL what decenders are. 🤓

Thanks @Edtadros - also asked this on the bugs themselves but just to be sure - these are only showing on old desktop browsers, right? Priority would probably increase if it was both desktop & mobile.

@ovasileva I only tested this on IE11. I can test on older browsers if you think it is necessary. I tested it on the browsers mentioned here T247146#5962575 and everything looks fine for both the edit icon size as well as the decender characters.