Page MenuHomePhabricator

Infobox breaking toc in new print styles
Closed, ResolvedPublic2 Story Points

Description

Background

The infobox is sometimes breaking the TOC.

Steps to recreate

  1. Go to http://reading-web-staging.wmflabs.org/wiki/Mini_Hatch using Firefox or Chrome
  2. Print article

expected behavior: there should not be a gap between the table of contents
observed behavior: the infobox break the table of contents

Proposed solution

Apply

#toc { overflow: hidden; }.

Related Objects

Event Timeline

ovasileva created this task.Sep 4 2017, 3:06 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 4 2017, 3:06 PM
ovasileva triaged this task as High priority.Sep 4 2017, 3:06 PM
Jdlrobson updated the task description. (Show Details)Sep 5 2017, 2:25 PM
Jdlrobson added a subscriber: Jdlrobson.

Wouldn't applying a max-width e.g. 50% be better than a overflow:hidden?
The issue here seems to be the table of contents + infobox have greater combined width than the page itself.

ovasileva updated the task description. (Show Details)Sep 5 2017, 4:54 PM
Niedzielski added a subscriber: Niedzielski.

We think this task is similar to T175005 but may be distinct. We're going to try and fix this issue first with the proposed solution and understand why the word wrapping is busted on list items before approaching the parent.

To do:

  • Talk with @Nirzar about what the two-column design should look like.
  • Point this task.

two column design?

i think if the overflow fixes the gap problem, let's go with that

This seems like the same as https://phabricator.wikimedia.org/T175008#3580659 - I suggest we think of them the same - in terms of how we deal with elements floated left and elements floated to the right - e.g. should we restrict them into columns?

The issue here is the table of contents. The table of contents is attempting to take up the whole width of the screen. The infobox which appears before it in the HTML is floated right so will appear above it when possible. Applying overflow: hidden will fix this but I think this is better fixed by applying the solution in https://phabricator.wikimedia.org/T175008#3584391

Change 376266 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/Vector@master] Do not allow certain content to span entire page

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

So the above patch attempts to fix this by defining a fixed width for infoboxes, thumbs and table of contents - and I've put it on staging for you to test.
Note now that the table of contents appears after infobox rather than floated left. This is because of page-break-before: avoid; - do we want to drop that rule as part of this work?

Not sure if this is the best way to go - seems the toc fixed-width is now adding a lot of white space.

Examples:
http://reading-web-staging.wmflabs.org/wiki/Cat
http://reading-web-staging.wmflabs.org/wiki/Tenochtitlan

Not sure if this is the best way to go - seems the toc fixed-width is now adding a lot of white space.

This is not due to the fixed width - it's because of page-break-before: avoid; - as I mentioned above can easily drop this to fix this problem but I wanted to check if we intended TOC to behave like that.

Nirzar added a comment.Sep 6 2017, 5:00 PM

This is because of page-break-before: avoid;

let's get rid of page break avoid...

So the current solution constraints infoboxes, floated thumbnails and table of contents to 180pt. I'm not sure if this is a sensible constraint (I got to it via trial and error) - but it seems to be half of the printed output and worked on the handful of articles I tested on. It does however mean that if content is greater than 180pt it may be clipped or behave oddly so if we merge this we may find follow up cases we need to deal with.

This will also take care of T175008. Is this a solution we want to go ahead with?

Does anyone have any other potential solutions that may be better? cc @bmansurov @Niedzielski @pmiazga

@Jdlrobson, hi Jon. Sorry for the noob question but why do we need display: table-cell on .tocnumber, .toctext? When I turn this rule and width: 180pt off, it seems to flow as I would expect.

ovasileva moved this task from Triage to Needs Analysis on the Proton board.Sep 6 2017, 6:59 PM

@Jdlrobson, hi Jon. Sorry for the noob question but why do we need display: table-cell on .tocnumber, .toctext

It looks like this is used to control how the table of contents displays on long lines. I don't know how attached we are to that. We could easily disable this in mobile.



And yes this is related and another possible solution. The problem being that if the table does not express a width, it will consider the table of contents a table and assume it cannot break the content flow.

That won't help with T175008 though (hence why I think it's good to think of these 2 problems together).

MBinder_WMF set the point value for this task to 2.Sep 11 2017, 5:17 PM
bmansurov added a comment.EditedSep 11 2017, 5:44 PM

I noticed we don't have this problem in Vector and the difference between Vector and the print styles is that we're displaying the table of contents as table in vector and block in print styles. Any reason why we chose to use block in print styles? If no, then the issue seems to be fixable by switching to table.

I also think that setting an arbitrary width of 180pt may cause problems down the road when infoboxes will be styled with template styles. We'll lose the flexibility of applying different widths depending on the width of the printed page.

phuedx added a subscriber: phuedx.

As discussed, I'm moving this back to To Do as there's no one working on it.

bmansurov added a project: Vector.

Change 379605 had a related patch set uploaded (by Bmansurov; owner: Bmansurov):
[mediawiki/skins/Vector@master] Print styles: fix breaking of table of contents

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

Change 376266 abandoned by Jdlrobson:
Do not allow certain content to span entire page

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

bmansurov reassigned this task from bmansurov to ABorbaWMF.Sep 25 2017, 4:45 PM

Change 379605 merged by jenkins-bot:
[mediawiki/skins/Vector@master] Print styles: fix breaking of table of contents

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

I've updated reading web staging for testing. Over to your Anthony.

Looks good now. Tried it on Chrome, Firefox, and Safari.

ovasileva closed this task as Resolved.Sep 26 2017, 12:39 PM

looks good to me!