Page MenuHomePhabricator

Avoid unintended wrapping on en.m.wikipedia.org "2016 Formula One season" article
Closed, ResolvedPublic3 Story Points

Description

As reported at https://en.wikipedia.org/wiki/Wikipedia:Village_pump_(technical)#Recent_changes_concerning_mobiles , flag icons and the text following them are rendering with an apparent newline on mobile web Wikipedia on one or more articles. See, for example, the following, as reported:

https://en.m.wikipedia.org/wiki/2016_Formula_One_season#Teams_and_drivers

As a reader, I want to see the flag images and and text on the same line, so that I can scan the flags or the text with ease.

Generally, we shouldn't be asking for across-the-board wikitext updates, of course!

At the conclusion of this task, we should have a code fix in mind (maybe so simple it's done?), plus communicate out what's happening next on the village pump topic.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 22 2016, 3:14 PM
dr0ptp4kt triaged this task as High priority.Aug 22 2016, 3:14 PM
dr0ptp4kt added a project: Readers-Web-Backlog.
dr0ptp4kt moved this task from Incoming to Product Owner Backlog on the Readers-Web-Backlog board.

We need to have a quick chat around this and come up with the right way to solve it.

Possible ways to resolve on top of my head:

  • Don't apply lazy loading images to images of this size
  • Fix on wiki
  • Re-explore display: block style on images.
dr0ptp4kt renamed this task from Avoid unintended wrapping on en.m.wikipedia.org "2016 Formula One season" article to [SPIKE: 1 hour] Avoid unintended wrapping on en.m.wikipedia.org "2016 Formula One season" article.Aug 22 2016, 4:13 PM
dr0ptp4kt added a project: Spike.

display block on the lazy images placeholder is there for avoiding reflows in thumbnails with captions.

Why are thumbnails block and this ones inline-block? how can we respect the original display?

Also careful with the styling changes, given this is live everywhere now.

dr0ptp4kt updated the task description. (Show Details)Aug 22 2016, 4:48 PM

According to https://gerrit.wikimedia.org/r/#/c/294689/ the easiest fix would be changing back to inline-block and adding overflow: hidden.

I think the simplest solution that will solve this case is add a rule

span.lazy-image-placeholder { display: inline-block;}

@bmansurov that works too but a tad more risky since it will impact all images :)

(I've temporarily fixed this on wiki via Mobile.css. I can remove it once we've got this fixed in the code)

I think the simplest solution that will solve this case is add a rule
span.lazy-image-placeholder { display: inline-block;}

That will cause T135430#2384461

@bmansurov I don't see why. These were wrapped in <a> tags not <span>. Can you elaborate with a test case?

Moved back to "TODO" since it has no assignee.

@bmansurov I don't see why. These were wrapped in <a> tags not <span>. Can you elaborate with a test case?

Sure. Similarly, the flags are spans that are wrapped in links.

Sorry, I meant span .lazy-image-placeholder { display: inline-block;} (ie. only do this if the placeholder itself is wrapped in a span)

This is a snippet from the link in the description:

<span class="flagicon">
	<a href="/wiki/Germany" title="Germany">
		<noscript>&lt;img alt="Germany" src="//upload.wikimedia.org/wikipedia/en/thumb/b/ba/Flag_of_Germany.svg/23px-Flag_of_Germany.svg.png" width="23" height="14" class="thumbborder" data-file-width="1000" data-file-height="600"&gt;</noscript>
		<span class="lazy-image-placeholder loaded" style="width: 23px;height: 14px;" data-src="//upload.wikimedia.org/wikipedia/en/thumb/b/ba/Flag_of_Germany.svg/23px-Flag_of_Germany.svg.png" data-alt="Germany" data-width="23" data-height="14" data-srcset="//upload.wikimedia.org/wikipedia/en/thumb/b/ba/Flag_of_Germany.svg/35px-Flag_of_Germany.svg.png 1.5x, //upload.wikimedia.org/wikipedia/en/thumb/b/ba/Flag_of_Germany.svg/46px-Flag_of_Germany.svg.png 2x" data-class="thumbborder">
			<img class="thumbborder" width="23" height="14" src="//upload.wikimedia.org/wikipedia/en/thumb/b/ba/Flag_of_Germany.svg/23px-Flag_of_Germany.svg.png" alt="Germany" srcset="//upload.wikimedia.org/wikipedia/en/thumb/b/ba/Flag_of_Germany.svg/35px-Flag_of_Germany.svg.png 1.5x, //upload.wikimedia.org/wikipedia/en/thumb/b/ba/Flag_of_Germany.svg/46px-Flag_of_Germany.svg.png 2x">
		</span>
	</a>
</span>

The placeholder is wrapped in a link. Applying your suggestion wouldn't fix the problem as the structure of HTML is not what you'd expect.

The placeholder is wrapped in a link which is wrapped in a span, thus there is an intention there that should be inlined rather than block, no?

I'd guess so, although we never know as spans maybe styled to be displayed as blocks onwiki. I guess I'm trying to say is that your solution is good, we just need to add overflow: hidden to avoid the issue linked above.

I've got https://phabricator.wikimedia.org/T143558#2572767 running on enwiki so let's keep an eye out there and see if we see any further issues and if not formalise that rule.

TheDJ added a subscriber: TheDJ.Aug 24 2016, 7:59 AM

People report that the flag images are vertically and horizontally cut off significantly.

@TheDJ What happens is that the image placeholder takes the height&width from the img html which says to be 23x15, but that ends up being false.

The flag img is 23x15 in the html attrs, the in-dom dimensions are 27x17.

Apparently there's a 1px border, which adds 2px to height and width, and there's some inline stylesheet modifying the image dimensions artificially:

.flagicon img {
    min-width: 25px;
}

Which accounts for the 27px width.

As a result, when using display: inline-block; overflow: hidden; the placeholder ends up hiding 5px horizontally and 2px vertically.

@Jdlrobson the current style applies to all images, thumbnails and flags, but it still not fixes flags, see previous comment.

I think this case warrants a more specific selector, with the idea you outlined in T143558#2577501

span .lazy-image-placeholder {
  display: inline-block;
}

And avoid overflow: hidden; given the reflows for which that was suggested happen on thumbnails.

We may find this affects some other corner cases too, so it maybe a good idea to actually target content fixes by using .flagicon to not inadvertently affect other images.

.flagicon .lazy-image-placeholder {
  display: inline-block;
}

My personal take on it would be using .flagicon with a comment linking to this very issue. It is not a general solution, but it is a good one for now until we figure out other broken patterns in content and come up with general rules to fix them.

I've tested it on-wiki and it seems to work fine.

If somebody that can access Mobile.css can change the current

span.lazy-image-placeholder {
  display: inline-block;
}

Into

.flagicon .lazy-image-placeholder {
  display: inline-block;
}

It would be great to unbreak what's semi-broken now.

Meanwhile we'll discuss the approach to the patch to the codebase ASAP.

Given .flagicon is an editor construct I would prefer we go with the generic selector to avoid having yet another editor class in MobileFrontend:

span .lazy-image-placeholder {
  display: inline-block;
}

I've updated MediaWiki:Mobile.css to use flagicon in the meantime.

Change 306479 had a related patch set uploaded (by Jdlrobson):
Don't treat certain image placeholders as block

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

TheDJ added a comment.EditedAug 24 2016, 9:23 PM

Apparently there is a 1 pixel border

Yes there is, so you are copying the wrong dimensions. You want outerWidth/Height and not the contentWidth/Height

and there's some inline stylesheet modifying the image dimensions artificially:

Yes, that is Mobile.css, which needs to override

.content a > img {
max-width: 100% !important;
}

which is used for responsive images, but responsive images are impossible inside tables, so we made this 'as good as it gets hack' remember.. There is a ticket about it somewhere.

TheDJ added a comment.Aug 24 2016, 9:27 PM

In general however, messing with the position of a image inside the DOM tree accross our different output as there is desktop, parsoid etc, seems like a bad and not very well maintainable idea, you are guaranteed to break child selectors. Like for instance, .content a > img which is no longer applied to any lazy loaded image it seems...

We used to replace .lazy-image-placeholder with the lazy loaded image but we hit issues with that. I'd have to remind myself why. I'd like to address problems on a case by case basis. @TheDJ could you file a bug for ".content a > img" if you are seeing that as a problem on certain articles?

So how do people feel about https://gerrit.wikimedia.org/r/306479 ... ?

Bmansurov left me a comment I didn't quite understand. I think he's talking about how the flagicon's are clipped.

For this particular case the following rule should suffice:

span .lazy-image-placeholder {
display: inline;
    overflow: hidden;
    height: auto !important;
}

This doesn't have the clipped (due to height) and due to display inline doesn't introduce a phantom gap.

@Jdlrobson
Compare

(after applying your patch) with
(what it should look like). Did you notice the space after the flag?

phuedx added a subscriber: phuedx.EditedAug 25 2016, 9:23 PM

I think that in order to come up with a general approach, we need a larger, more varied corpus.

Edit

… and that, perhaps, fixing the general problem is out of scope for this 1 hour spike?

Using inline will work provided the image has a width and height. I'm dealing with that as part of the fix for T143768

Change 306479 merged by jenkins-bot:
Don't treat certain image placeholders as block

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

phuedx reassigned this task from Jdlrobson to ovasileva.EditedAug 31 2016, 10:53 AM
phuedx added a subscriber: ovasileva.

Over to you @ovasileva/@dr0ptp4kt.

@Jdlrobson: How can this be tested/signed off?

MBinder_WMF renamed this task from [SPIKE: 1 hour] Avoid unintended wrapping on en.m.wikipedia.org "2016 Formula One season" article to Avoid unintended wrapping on en.m.wikipedia.org "2016 Formula One season" article.Aug 31 2016, 5:16 PM
MBinder_WMF set the point value for this task to 3.
ovasileva closed this task as Resolved.Aug 31 2016, 7:45 PM