Page MenuHomePhabricator

Justified text appearing inconsistent due to large infobox when printing with new styles in firefox
Closed, ResolvedPublic3 Story Points

Description

Background

Justified text seems to break in certain places when printing articles in Firefox 55.0.3

Steps to recreate

  1. Go to http://reading-web-staging.wmflabs.org/wiki/Mini_Hatch
  2. Print the article

Expected behavior: text should justify normally


Observed behavior: justification breaks text

QA Plan/Notes

Test printing a variety of rich articles (with infoboxes and pictures in the lead/first section) on the Beta Cluster in landscape and portrait mode.

If you're using Chrome, you can speed up testing printing using the DevTools by emulating the print media query: https://developers.google.com/web/tools/chrome-devtools/device-mode/emulate-mobile-viewports

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 5 2017, 11:54 AM
ovasileva renamed this task from Justified text breaking in firefox to Justified text breaking when printing with new styles in firefox.Sep 5 2017, 11:54 AM
ovasileva updated the task description. (Show Details)
ovasileva triaged this task as High priority.Sep 5 2017, 11:56 AM
ovasileva renamed this task from Justified text breaking when printing with new styles in firefox to Justified text appearing inconsistent due to large infobox when printing with new styles in firefox.Sep 5 2017, 4:25 PM
Jdlrobson assigned this task to Nirzar.Sep 5 2017, 4:32 PM
Jdlrobson added a subscriber: Jdlrobson.

Nirzar the issue here is that the thumbnail is floated to the left and the infobox is floated to the right and their combined widths leave a bit of space in the middle for text.

Possible solutions include:

  • Limiting floated left/right elements to a certain amount of space e.g. 50% of the screen / 33% of the screen
  • Clearing content to a new line

Note: infobox widths tend to get defined in inline styles so we should probably test on a few articles before committing to a solution. If an infobox has an element greater in width than itself that content will be clipped when we define a maximum width.

We can experiment on staging with different solutions.... let us know how to proceed!

Limiting floated left/right elements to a certain amount of space e.g. 50% of the screen / 33% of the screen

I like this. let's use display blocks if the elements take up more than 50%

but that wouldn't solve this problem, would it? since this is 2 elements together taking up more than 50%

but that wouldn't solve this problem, would it? since this is 2 elements together taking up more than 50%

What I'm saying is we can restrict it to a max width of 50% -- at least for infobox, table of contents and thumbnail.

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

ovasileva lowered the priority of this task from High to Normal.Sep 6 2017, 3:10 PM
ovasileva raised the priority of this task from Normal to High.
ovasileva moved this task from Triage to Needs Analysis on the Proton board.Sep 6 2017, 6:59 PM

If I understand correctly, this is the issue specifically:

That is, the wrapping behavior under certain widths on certain articles is undesirable because it leaves orphans. I tried changing the clear: left behavior with the idea that we could do this with media queries:

Wide (off):


Thin (on):

Narrow (off):

But I guess this would be problematic across a diversity of articles.

Jdlrobson added a subscriber: MBinder_WMF.

Why has this and T174957 been moved to do? The work has been done (so this probably should be code review), but I'm reluctant to seek review for this until https://phabricator.wikimedia.org/T174957#3585819) has some clearer answers. @MBinder_WMF how would we capture this on a kanban board?

Thanks for flagging, @Jdlrobson. It's possible Olga moved it from Upcoming ("getting ready") to To Do ("obviously ready") because work was already being done, and left it to the discretion of the team to columnize (I'm speculating). It seems sensible that if "the work has been done" then it is, in fact, in progress, and #readers-web-kanban-board is the appropriate place for it while we try to figure out the review approach.

@ovasileva Should the team estimate this before it enters To Do? If not, it would be useful to have an explanation in a comment saying that it is ready anyway. :)

@MBinder_WMF, @Jdlrobson exactly. We had a pause during sprint planning before estimation, but agreed that the work would go into this sprint. Thus - moving to to-do. I agree that we should have an estimate on this and T174957: Infobox breaking toc in new print styles.

MBinder_WMF set the point value for this task to 3.Sep 11 2017, 5:10 PM

Change 377317 had a related patch set uploaded (by Bmansurov; owner: Bmansurov):
[mediawiki/skins/Vector@master] Print styles: set minimum width for paragraphs

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

bmansurov added a subscriber: bmansurov.

I've pushed an alternative solution, which I think is cleaner and scales to all problematic articles. ^

@Nirzar, do you think the minimum width of 120pt is enough for paragraphs in this situation? If not, can you give a number so that I can update the above patch?

@bmansurov
is the solution to clear both sides of paragraphs? or we can jump on a call if this is a lot of back and forth

bmansurov added a comment.EditedSep 11 2017, 6:46 PM

When the space is less than 120pt:

When the space is more than 120pt:

phuedx added subscribers: Volker_E, phuedx.

@bmansurov's change looks fine (and has been +1'ed by @Volker_E) but we're waiting on @Nirzar to OK the 120 pt constant AFAICT.

coming up the magic number would be difficult, let's go with 120 for now.

bmansurov removed Nirzar as the assignee of this task.Sep 13 2017, 9:09 PM
bmansurov added a subscriber: Nirzar.

Change 377317 merged by jenkins-bot:
[mediawiki/skins/Vector@master] Print styles: set minimum width for paragraphs

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

phuedx claimed this task.Sep 15 2017, 8:15 AM

I need to add a QA plan.

phuedx reassigned this task from phuedx to ABorbaWMF.Sep 15 2017, 11:07 AM
phuedx updated the task description. (Show Details)
phuedx added a subscriber: ABorbaWMF.

Over to you, @ABorbaWMF!

Hello, I took a quick look and noticed a few things that I wanted to share. I tried a couple articles on Staging. Here is what I found:

Mini-Hatch

ChromeFirefox
Differences
On Chrome there is text between the images pg2On Firefox the infobox borders do not extend on infoboxes longer than one page pg3

Cassini-Huygens

ChromeFirefox
Differences
Text is squished next to info boxes on bothSame border issue as above, but a little more visible with the wide infobox
phuedx reassigned this task from ABorbaWMF to bmansurov.EditedSep 18 2017, 8:36 AM
phuedx moved this task from Needs QA to Needs More Work on the Readers-Web-Kanbanana-Board-Old board.

@bmansurov: Could you respond to @ABorbaWMF's report (T175008#3611413)?

Looks good to me. The first issues is tracked under T174955: Infobox styling disappears in firefox. For the second I believe @bmansurov suggested a fix by limiting the width of the infobox in https://phabricator.wikimedia.org/T172184#3567800 - I think this should be tracked separately though. I think for this one, we're ready for signoff. I'll let @Nirzar and @bmansurov confirm.

Mostly looks good. I'm curious why in the first scenario the text doesn't appear in between the image and the infobox in Firefox. From the screenshots it looks like the Firefox is reserving bigger margins, thus reducing the space for text.

@ABorbaWMF could we re-do the first scenario with the same margins for PDFs? Could we also remove the header/footer added by the browsers?

bmansurov reassigned this task from bmansurov to ABorbaWMF.Sep 18 2017, 5:35 PM

@bmansurov I'm not sure I understand. I can change the margins a bit by choosing different options while printing. Are you looking for something like 'actual size'? I am also not sure how to remove the header/footer from my end. Admittedly, I don't know much about PDF generation, so please forgive me if I am missing something obvious. Thanks

@ABorbaWMF thanks. There are a couple of options in Firefox's Options tab of the print dialog. I've tweaked them to remove the header/footer (things like date, URL, etc.). I also set the page size to US Letter (without changing the margins -- unlike what I asked for). I still see the problem, but I think that's because the infobox is unusually large. And that issue is tracked in another task. So I think we're good here.

Edit:
^ Was on my development environment. I've used the staging environment to create PDFs using the same page size and same margins (0.26, 0.24, 0.24, 0.26 inches). Firefox still ignores these settings and outputs large margins. When I set to ignore scaling and fit everything in one page, Firefox behaved correctly. Here are the results:

  • Chrome

  • Firefox

  • Firefox (scaling ignored):

So, as far as we're concerned, the solution given to the problem works. However, Firefox has issues with margins and causes all kinds of discrepancies when a page is printed.

Looks good to me too. @bmansurov - to double-check, the default for firefox would be the scaling ignored version, right? Over to @Nirzar for signoff

I don't think the default is to fit everything on the page (i.e. scaling ignored).

phuedx removed ABorbaWMF as the assignee of this task.Sep 19 2017, 5:06 PM

Since the task is now in Ready for Signoff.

Nirzar closed this task as Resolved.Sep 21 2017, 5:26 PM

Macro votecat: looks  good

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

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