Page MenuHomePhabricator

[Bug] Lazy loaded image placeholders larger than viewport cause horizontal scrollbar on small screens
Closed, ResolvedPublic

Description

Steps to Reproduce

  1. Set the screen width 450px.
  2. Visit https://reading-web-staging.wmflabs.org//w/index.php?title=Barack_Obama&oldid=852588387
  3. Open DevTools and disable the internet connection (go offline).
  4. Scroll to the "Legacy" section.
  5. Open "Legacy" section
  6. Observe gray image placeholder

Expected Results

  • Placeholder is same dimensions as the image and constrained to the image viewport

Actual Results

  • Placeholders are the unconstrained image dimensions
    en.m.wikipedia.org_wiki_Barack_Obama.png (827×450 px, 64 KB)
    , is larger than the viewport and it is possible to horizontally scroll

Environments Observed

Browser Version

  • Chromium v67.0.3396.99

OS Version

  • Ubuntu v18.04 64b

Device Model

  • Desktop

Device Language

  • English

Developer notes

The issue is related to the use of display: flex on the .thumbinner element. Removing display: flex or assigning a max-width to the child element makes the issue go away.

I notice that setting the flex-direction to column (currently default row) makes this go away.

.content a > img, .content a > .lazy-image-placeholder, .content noscript > img {
max-width: 100%;
}

needs to be expanded to include a.image

The image in question has width 512px set.

Event Timeline

Jdlrobson renamed this task from [Bug] Lazy loaded image placeholders cause horizontal scrollbar on small screens to [Bug] Lazy loaded image placeholders larger than viewport cause horizontal scrollbar on small screens.Jul 30 2018, 3:01 AM
Jdlrobson updated the task description. (Show Details)
Jdlrobson updated the task description. (Show Details)
Jdlrobson moved this task from Needs Prioritization to Upcoming on the Readers-Web-Backlog board.

I'm having trouble replicating this bug. This is how it looks on Chrome 67.0.3396.99 on Mac OS High Sierra 10.13.5:

Screen Shot 2018-07-31 at 5.20.10 PM.png (1×860 px, 364 KB)

@nray, I can no longer reproduce this issue either.

Discussed taking some time to reproduce and look into potential solutions prior to tackling the issue - we estimated around 4hrs for this investigation

ovasileva renamed this task from [Bug] Lazy loaded image placeholders larger than viewport cause horizontal scrollbar on small screens to [Bug] [Spike 4hrs] Lazy loaded image placeholders larger than viewport cause horizontal scrollbar on small screens.Aug 1 2018, 4:14 PM

Change 449999 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Set flex-direction for thumbnails to avoid overflow issues

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

Jdlrobson added a subscriber: Jdlrobson.

I can replicate and fix seems straightforward to me. Will chat with devs to work out what's going on here.

I can repro again today. I felt confident at my go yesterday so I'm not sure if reproduction is flaky or I'm flaky. Beware!

Change 449999 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Set flex-direction for thumbnails to avoid overflow issues

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

Jdlrobson renamed this task from [Bug] [Spike 4hrs] Lazy loaded image placeholders larger than viewport cause horizontal scrollbar on small screens to [Bug] Lazy loaded image placeholders larger than viewport cause horizontal scrollbar on small screens.Aug 2 2018, 1:06 PM
Jdlrobson removed a project: Patch-For-Review.
Jdlrobson reassigned this task from ABorbaWMF to alexhollender.
Jdlrobson updated the task description. (Show Details)
Jdlrobson added subscribers: alexhollender, ABorbaWMF.

I've loaded this on reading-web-staging

@alexhollender take a look at this. You should check a few other images to check nothing looks out of the ordinary because of this change.

@Jdlrobson - is this connected to another task? Wondering why it's currently on the board...

@ovasileva a patch has been merged which fixes the problem so it should go through our QA, right?

I guess my question then is: why are we working on this right now?

Restricted Application changed the subtype of this task from "Deadline" to "Task". · View Herald TranscriptAug 22 2018, 1:18 PM

Hrrm. I'm seeing the same thing that @nray saw in T200518#4467436 on both reading-web-staging and readers-web-master, i.e. not what @alexhollender saw in T200518#4499137.

Edit

Browser: Chrome (68.0.3440.106)
OS: macOS High Sierra (10.13.6)

I met with @Jdlrobson about this after today's kickoff ritual. We tested this in production (enwiki), and proved to ourselves that the fix works by following the QA steps in the description with the window set to 450px and 375px widths.

Here's what we saw:

WidthResultNotes
375px
t200518.png (400×427 px, 30 KB)
-
450px
Screen Shot 2018-07-31 at 5.20.10 PM.png (1×860 px, 364 KB)
I'm using @nray's screenshot here as, per T200518#4523243, I was seeing that already.
ovasileva claimed this task.

looks good, thanks for the QA @phuedx and @Jdlrobson!