Page MenuHomePhabricator

Vector: Use semantic HTML5 elements where applicable
Closed, ResolvedPublic3 Estimated Story Points

Description

With T248137 in, we're able to use semantic HTML5 elements without restrictions.
Elements aimed for are header, main, nav and footer.

Example specification for nav at https://developer.mozilla.org/en-US/docs/Web/HTML/Element/nav

Additionally we can remove ARIA landmark role definitions when element automatically commmunicates landmark.


https://developer.mozilla.org/en-US/docs/Web/HTML/Element/aside was considered as well, but not seen as correct semantic representation of anything currently in Vector. Given its “content is only indirectly related to the document's main content” and article tools are directly related to the content for example

QA steps

  • Ensure there are no new layout regressions on beta cluster in comparison to production in Internet Explorer 8 and 11 before and after this patch. There might be certain layout inconsistencies already in modern Vector with either of those browsers
    • Ensure that both versions of Vector ?useskinversion=1 and ?useskinversion=2 are tested before after in both browsers
  • Ensure "Reader" mode works in Firefox, Safari and Mobile Safari in production. Wikipedia's domain name quite regularly gets special-cased to optimise or fix things they run into.

+++ This bug was initially created as a clone of Bug #61615 (now T63615) +++
Version: 1.24rc
Severity: enhancement

QA Results - Beta

ACStatusDetails
1T66477#6215871
2T66477#6215871

QA Results - Prod

ACStatusDetails
1T66477#6255468
2T66477#6255468

Related Objects

StatusSubtypeAssignedTask
ResolvedGoalovasileva
OpenNone
Resolvedovasileva
ResolvedSpikeovasileva
ResolvedSpikephuedx
Resolvedovasileva
OpenSpikeNone
ResolvedSpikeovasileva
Resolvedovasileva
ResolvedBUG REPORTmatmarex
Resolvedovasileva
ResolvedJdlrobson
Resolvedphuedx
Resolvednray
ResolvedMayakp.wiki
ResolvedMayakp.wiki
Stalledovasileva
OpenNone
ResolvedEdtadros
OpenNone
OpenNone
OpenNone
DuplicateNone
ResolvedNone
Resolvedovasileva
Resolvedsgrabarczuk
ResolvedBUG REPORTovasileva

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

(Sorry for the noise)

Volker_E renamed this task from Wrap MediaWiki toolbar menu to HTML5 <nav> and <aside> tag to Vector: Wrap MediaWiki toolbar menu to HTML5 <nav> and <aside> tag.May 7 2020, 1:46 AM
Volker_E edited projects, added Desktop Improvements; removed MediaWiki-Interface.

Change 594819 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/skins/Vector@master] Use semantic HTML5 elements where applicable

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

Volker_E added a comment.EditedMay 7 2020, 10:06 PM

Given the current mix of scope in Vector sidebar, having for example page tools in the panel for the portal, aside is not the right element to choose.
Therefore narrowing scope of this task.

We will watch structural elements application in (modern) Vector and refine if useful with the further steps taken in Desktop Improvements project.

Volker_E renamed this task from Vector: Wrap MediaWiki toolbar menu to HTML5 <nav> and <aside> tag to Vector: Use HTML5 <nav> where applicable.May 7 2020, 10:08 PM
Volker_E updated the task description. (Show Details)

Change 594819 merged by jenkins-bot:
[mediawiki/skins/Vector@master] Use semantic HTML5 elements where applicable

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

Volker_E updated the task description. (Show Details)May 7 2020, 10:18 PM
Volker_E renamed this task from Vector: Use HTML5 <nav> where applicable to Vector: Use semantic HTML5 elements where applicable.May 7 2020, 10:37 PM
Volker_E updated the task description. (Show Details)
Iniquity added a subscriber: Iniquity.
Krinkle removed a subscriber: Krinkle.May 9 2020, 2:24 AM
Krinkle added a comment.EditedMay 9 2020, 2:29 AM

Once this rolls out, be sure to also test "Reader" mode in Firefox, Safari and Mobile Safari. Not just locally or beta, but also in production. Wikipedia's domain name quite regularly gets special-cased to optimise or fix things they run into.

Volker_E added a subscriber: Johan.

@Johan Let's add a user notice:

Over-qualified CSS selectors of portals in Wikimedia skins have been changed. <code>div#p-personal</code>, <code>div#p-navigation</code>, <code>div#p-interaction</code>, <code>div#p-tb</code>, <code>div#p-lang</code>, <code>div#p-namespaces</code> or <code>div#p-variants</code> are now all removed of the <code>div</code> qualifier, as in for example it is <code>#p-personal, #p-navigation …</code>. This is so the skins can use HTML5 elements. If your gadgets or user styles used them you will have to update them.

Volker_E claimed this task.May 11 2020, 8:18 PM
Volker_E added a subscriber: Edtadros.
Volker_E updated the task description. (Show Details)May 11 2020, 9:32 PM

@Johan User notice task has actually been split out into T252467 to be even clearer and better addressing gadget developers and users script/stylesheet editors.

Change 595962 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/Vector@master] Use semantic HTML5 elements where applicable with minimal disruption

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

Change 595962 abandoned by VolkerE:
Use semantic HTML5 elements where applicable with minimal disruption

Reason:
Superseded by I540d9a41fc7fd580c5d61b90480e8745ae145850

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

Jdlrobson moved this task from Needs triage to Technical on the Vector board.May 15 2020, 4:59 PM

Change 596312 had a related patch set uploaded (by Krinkle; owner: VolkerE):
[mediawiki/skins/Vector@master] Use semantic HTML5 elements where applicable

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

Izno added a subscriber: Izno.May 19 2020, 1:58 AM
Izno added a comment.May 19 2020, 2:23 AM

Kind of curious to know why <article> isn't in the list. Seems like an obvious inclusion. :)

Volker_E added a comment.EditedMay 19 2020, 4:49 AM

@Izno Would you expand what you're missing without [[ https://developer.mozilla.org/en-US/docs/Web/HTML/Element/article | article ]]? We feature main and there's no obvious semantical win in my experience to additional define article.
On a blogging system like WordPress that might be more useful with a number of article(s) (excerpts) per view.

We are down down to 261 pages managed by 214 users across all wikis. Pending a reply to T252467#6197821 I think we should push ahead with this change.

Jdlrobson raised the priority of this task from Low to Medium.Jun 5 2020, 8:48 PM

Change 596312 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/skins/Vector@master] Use semantic HTML5 elements where applicable

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

For completion, above patch still features role ARIA attributes even though the native elements imply them. We will leave them around for some time to ensure CSS selectors building on top of them won't break.
role="contentinfo is a special case, as there's a VoiceOver bug, that doesn't expose footer as contentinfo.

Change 596312 merged by jenkins-bot:
[mediawiki/skins/Vector@master] Use semantic HTML5 elements where applicable

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

Jdlrobson reassigned this task from Volker_E to Edtadros.Jun 9 2020, 8:32 PM
Edtadros reassigned this task from Edtadros to Volker_E.EditedJun 11 2020, 4:23 PM

Test Result - Beta

Status: ✅ Pass
Environment: beta
OS: macOS Catalina
Browser: Chrome
Device: MBP
Emulated Device: NA

Test Artifact(s):

QA steps

✅ AC1: Ensure there are no layout regressions on beta cluster in comparison to production in Internet Explorer 8 and 11.
IE8 on Win7 has some regressions but per T66477#6216167 they are not related to this task.

IE8IE11
Beta
enwiki

✅ AC2: Ensure "Reader" mode works in Firefox, Safari and Mobile Safari in production. Wikipedia's domain name quite regularly gets special-cased to optimize or fix things they run into.

FirefoxSafariMobile Safari
Full page screen grab from Safari is a PDF not an image.
Edtadros updated the task description. (Show Details)Jun 11 2020, 4:23 PM

QA Steps need to be updated to look for before/after in IE 8 and in IE 11.
The issues surfaced are issues, but from my understanding not related to the specific change in this task, but earlier breakages.

Volker_E reassigned this task from Volker_E to Edtadros.Jun 11 2020, 5:21 PM
Volker_E updated the task description. (Show Details)
Volker_E removed a project: Patch-For-Review.
ovasileva set the point value for this task to 3.Jun 11 2020, 5:35 PM
Edtadros updated the task description. (Show Details)Jun 15 2020, 3:05 PM
Edtadros added a subscriber: ovasileva.

@ovasileva, I'm moving this to Needs QA in Prod 🤠

Nirmos added a subscriber: Nirmos.Jun 17 2020, 9:59 PM

Change 606838 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/skins/Vector@master] [CSS] Set display: block for main element in IE

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

Demian added a comment.EditedJun 22 2020, 4:07 PM
@Jdlrobson wrote on gerrit:

The HTML5Shiv library which we already use has support for this: https://github.com/aFarkas/html5shiv#html5shivcss
We should apply this rule in core as part of runnnig the shiv, otherwise we're going to run into this bug again when other skins migrate.

I wondered why that wasn't done.

"In most cases a webpage author should include those basic styles in his normal stylesheet to ensure older browser support (i.e. Firefox 3.6) without JavaScript." (ref)

Would you agree with adding these rules to core/resources/src/mediawiki.skinning/elements.css?

@Demian that sounds better yes. Although I'm not sure if it's better to have core/resources/src/mediawiki.skinning/elements.css or a new html5 feature on ResourceLoaderSkinModule.

Change 607077 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/core@master] [CSS] Add HTML5 shiv rules to fix unknown elements in IE

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

I'm not sure if it's better to have core/resources/src/mediawiki.skinning/elements.css or a new html5 feature on ResourceLoaderSkinModule.

That's a forward-looking idea, though I don't see myself investing the effort into configuring RL. IE11 will be around for a while and I don't think we would conditionally load just these rules for IE with <!--[if IE]>.

Change 606838 abandoned by Aron Manning:
[CSS] Set display: block for main element in IE

Reason:
Rules added to core in patch: https://gerrit.wikimedia.org/r/c/mediawiki/core/ /607077

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

A separate phabricator ticket(s) should capture the IE11 issue per usual practice. Already some good conversation is occurring on Gerrit and here, based on new information from testing and we need to reconsider the priority and how it relates to deployment.

A separate phabricator ticket(s) should capture the IE11 issue per usual practice.

T256092: [Modern Vector] Fix broken rendering of `main` and element in IE9-11

reconsider the priority and how it relates to deployment.

Affects only modern layout in IE9-11. AFAICT it's not yet enabled on production wikis.
It makes the content unreadable, however, thus it's best not to leave for the last minute.

Change 606838 restored by Aron Manning:
[CSS] Set display: block for main element in IE

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

Edtadros reassigned this task from Edtadros to ovasileva.Jun 25 2020, 5:41 AM

@ovasileva Please take a look at the issues in AC1.

Test Result - Prod

Status: ❓ Please check before closing
Environment: enwiki
OS: macOS Catalina
Browser: Chrome
Device: MBP
Emulated Device: IE8-11 on Windows 7 using Browserstack

Test Artifact(s):

QA steps

❓ AC1: Ensure there are no new layout regressions in production in Internet Explorer 8 through 11 for Vector ?useskinversion=1 and ?useskinversion=2. There might be certain layout inconsistencies already in modern Vector with either of those browsers.
//There were some inconsistencies as shown below. I'm leaving this as a ❓just to highlight this for either fixing or accepting as designed/expected.

There were no observable layout inconsistencies in IE8 through IE11 in production for useskinversion=1.
There were no observable layout inconsistencies in IE8 in production for useskinversion=2.
There were some observable layout inconsistencies in IE9 through IE11 in production for useskinversion=1.

useskinversion12
IE8
IE9
IE10
IE11

✅ AC2: Ensure "Reader" mode works in Firefox, Safari, and Mobile Safari in production. Wikipedia's domain name quite regularly gets special-cased to optimize or fix things they run into. Compare useskinversion 1 and 2.

FirefoxSafariMobile Safari
Reader mode works for useskinversion 1 and 2Reader mode works for useskinversion 1 and 2Reader mode works for useskinversion 1 and 2
Edtadros updated the task description. (Show Details)Jun 25 2020, 5:48 AM

Thank you for doing this tedious testing, @Edtadros! We just recently got to IE11...

There were some observable layout inconsistencies in IE9 through IE11 in production for useskinversion=1.

I believe you intended to write "useskinversion=2". This bug only affects modern layout.

Reported above: T66477#6242972
Fixed in T256092: [Modern Vector] Fix broken rendering of `main` and element in IE9-11
Should be merged any moment now.

ovasileva closed this task as Resolved.Jun 29 2020, 5:00 PM

All done here!

Additional remark, as it hasn't been clearly enough stated here. The VoiceOver bug that let's us keep role=contentinfo on footer has been informed by: