[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Page MenuHomePhabricator

[Modern Vector] Fix broken rendering of `main` and element in IE9-11
Closed, ResolvedPublic2 Estimated Story PointsBUG REPORT

Description

Follow-up to T66477: Vector: Use semantic HTML5 elements where applicable
IE9-11: Content is unreadable, shifted left, over the sidebar.

IE11-layout-main-main.png (646×1 px, 62 KB)

Cause

<main> element is display: inline, resulting in incorrect layout.
https://stackoverflow.com/questions/20094276/ie11-is-missing-user-agent-style-for-main-element-display-block

Broken elements:

main, dialog, template

Rest follows spec:

nav, header, footer, ... https://caniuse.com/#search=html%20elements

QA Results - Beta

ACStatusDetails
1T256092#6335440 This is passed per T256092#6339521
2T256092#6335440
3T256092#6335440

QA Results - Prod

ACStatusDetails
1T256092#6349210
2T256092#6349210
3T256092#6349210

Related Objects

StatusSubtypeAssignedTask
ResolvedGoalovasileva
ResolvedJdlrobson
Resolvedovasileva
ResolvedSpikeovasileva
ResolvedSpikephuedx
Resolvedovasileva
ResolvedSpikeVolker_E
ResolvedSpikeovasileva
Resolvedovasileva
ResolvedBUG REPORTmatmarex
Resolvedovasileva
ResolvedJdlrobson
Resolvedphuedx
Resolved nray
ResolvedMayakp.wiki
ResolvedMayakp.wiki
Stalledovasileva
DeclinedNone
ResolvedEdtadros
InvalidNone
DeclinedNone
ResolvedNone
Resolvedovasileva
ResolvedBUG REPORTovasileva

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Change 607077 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/core@master] [CSS] Add rules from HTML5shiv to fix unknown elements in IE9-11

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

Change 607157 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/skins/Vector@master] [modern] Fix broken rendering of main and dialog elements in IE9-11

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

Demian triaged this task as Medium priority.Jun 23 2020, 2:06 AM
Demian updated the task description. (Show Details)

Change 607077 merged by jenkins-bot:
[mediawiki/core@master] resourceloader: Add 'html5' feature to ResourceLoaderSkinModule for unknown elements in IE9-11

https://gerrit.wikimedia.org/r/c/mediawiki/core/ /607077

Change 607157 merged by jenkins-bot:
[mediawiki/skins/Vector@master] [modern] Fix broken rendering of main and dialog elements in IE9-11

https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/ /607157

This has been reverted without review in https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/608243
It is reasonable to look for a more generic solution in T256520, but we shouldn't break again what's been fixed before the alternative made it to the codebase.

Now IE11 is broken again and work cannot be tested in that browser.

Change 608113 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/core@master] mediawiki.skinning: Add 'normalize' module and 'normalize.less'

https://gerrit.wikimedia.org/r/c/mediawiki/core/ /608113

Change 608497 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/skins/Vector@master] [modern] Fix rendering of main element in IE10 & 11

https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/ /608497

From gerrit:

@Volker_E wrote:

A new RL module that goes into production would need to go through deprecation process of a full MW version, which is a huge overhead, that I wanted to prevent as we target all our products and 3rd parties better with a fine-tailored normalize approach.

Yes, that's reasonable, I'd have done the same and seeing the replacement is ready to be merged, there are no issues other than the need for a bit more forward planning.

If you recall, this was originally a quick fix a week ago without any resourceloader complication:
https://gerrit.wikimedia.org/r/c/mediawiki/core/+/607077/1

The professional approach that gives the best results and least disruption in handling such cases is to make a quick fix that can be removed anytime without constraints, while working on a more thought-out, long-term solution.
The normalize.less approach is IMO the best option in the long-term, however the unnecessary disruption due to the bug could have been avoided until this approach is stabilized.

I'm happy to help manifesting such staged approaches, in fact that's what I did on T254546 too, unfortunately that was rejected as well and there was no fix provided for a long time.

There seems to be a misunderstanding about urgency. The new Vector is not stable yet and not deployed on any wikis. As a result there is no urgency with any task or bug fix. Quick fixes are not necessary at this point, what's important is we build out long term solutions and lay down maintainable architecture.

The modifications to ResourceLoaderSkinModule are not quick fixes. They become subject to the deprecation process as soon as they are in deployed code. This was why it was important to act quickly.

There seems to be a misunderstanding about urgency.

Development on IE happens now. This bug was undermining that work, that being the reason I've reported and fixed it.
There's no misunderstanding, but instead developers' needs aren't taken into consideration. I wonder if that could be improved.

The modifications to ResourceLoaderSkinModule are not quick fixes.

I've referred to a "quick fix" in its original form when it was just piggybacked on 'elements.css' without RL involvement (ref), I've highlighted this fact explicitly in my previous comment (T256092#6266569).
Such misunderstandings often happen. Can I do something to improve the communication efficiency?

I agree it was important to remove the html5 ResourceLoader module before it gets on the train and we are burdened with a mandatory deprecation process.
My concern is that this could have been avoided by:

  1. Forward planning for the normalize-based solution (T256520), which was created the day before the RL module was merged, thus known about.
  2. Merging a minimal, immediate solution - that has no deprecation constraints - for the 2-4 weeks between the bug was discovered and T256520 will be merged: the first solution I've submitted. That temporary solution can be removed once unnecessary without strings attached.

@Volker_E I believe this is your task now.

Change 608113 merged by jenkins-bot:
[mediawiki/core@master] mediawiki.skinning: Add 'normalize' module and 'normalize.less'

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

Change 608497 merged by jenkins-bot:
[mediawiki/skins/Vector@master] [modern] Fix rendering of main element in IE10 & 11

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

There seems to be a misunderstanding about urgency.

There's no misunderstanding, but instead developers' needs aren't taken into consideration. I wonder if that could be improved.

What are “developers' needs” here. Please expand…

Change 609895 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/skins/Vector@master] [modern] Fix rendering of main element in IE10 & 11

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

Change 609895 merged by jenkins-bot:
[mediawiki/skins/Vector@master] [modern] Fix rendering of main element in IE10 & 11 (again)

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

Jdlrobson added a subscriber: ovasileva.

@ovasileva the IE9-11 issues should be fixed now - how do you want to handle QA? As part of this one or as part of a larger task involving the review of all legacy browsers prior to deployment?

Edtadros subscribed.

Test Result - Beta

Status: ❓ Need More Info
Environment: beta
OS: macOS Catalina
Browser: IE9, IE10, IE11
Device: MBP
Emulated Device: BrowserStack on Win7

Test Artifact(s):

QA steps

❓ AC1: IE 9
@Volker_E, the content is appearing correctly, however the sidebar expand/collapse button is not. If this is a separate issue, let me know and I will pass this AC. See below.

Screen Shot 2020-07-25 at 9.51.20 AM.png (1×1 px, 942 KB)

✅ AC2: IE10

Screen Shot 2020-07-25 at 9.52.43 AM.png (1×1 px, 897 KB)

✅ AC3: IE11

Screen Shot 2020-07-25 at 9.53.28 AM.png (1×1 px, 931 KB)

@Edtadros This IE9 misbehaviour is somewhat worrisome, but unrelated and should be filed in a separate task.

Volker_E renamed this task from [Modern Vector] Fix broken rendering of `main` and `dialog` elements in IE9-11 to [Modern Vector] Fix broken rendering of `main` and element in IE9-11.Jul 28 2020, 1:47 AM
Volker_E removed a project: Patch-For-Review.
ovasileva set the point value for this task to 2.Jul 29 2020, 4:11 PM

Test Result - Prod

Status: ✅ PASS
Environment: prod
OS: macOS Catalina
Browser: IE9, IE10, IE11
Device: MBP
Emulated Device: BrowserStack on Win7

Test Artifact(s):

QA steps

✅ AC1: IE 9
The sidebar button is acceptable per T256092#6339521

Jul-30-2020 09-42-19.gif (666×640 px, 2 MB)

✅ AC2: IE10

Jul-30-2020 09-43-11.gif (666×640 px, 2 MB)

✅ AC3: IE11

Jul-30-2020 09-45-01.gif (666×640 px, 2 MB)

Looks good! Resolving.