Closed Bug 1661663 Opened 4 years ago Closed 3 years ago

Popular sites prevent the print preview Margins menu from working (users need a way to override @page margins)

Categories

(Toolkit :: Printing, defect, P2)

defect

Tracking

()

RESOLVED FIXED
96 Branch
Tracking Status
firefox96 --- fixed

People

(Reporter: mbalfanz, Assigned: mstriemer)

References

Details

(Whiteboard: [print2020] [old-ui-] )

Attachments

(2 files)

STR:

  1. visit pinterest.com
  2. open the print dialog and adjust the print margins

ER: margins should be set and reflected in the print preview
AR: nothing changes in the print preview and output

pinterest was just one example. I noticed the same on other pages, like survey gizmo reports. Unfortunately, I couldn't find a common denominator yet.

Severity: -- → S2
Priority: -- → P2

(Also tagging jwatt, in case this ends up being a platform thing…)

Attached file A simplified test case (deleted) —

Looks like @page margin does cause this. (I am not sure whether it's correct or not)

Component: Printing → Printing: Setup
Product: Toolkit → Core
Summary: print margin settings has no effect on pinterest.com → print margin settings has no effect on pinterest.com and other sites
Whiteboard: [print2020_v81] → [print2020_v81][old-ui-]

Yes, this seems working as expected if the page overrides the margin.

From a user point of view, this behavior is problematic in 2 ways:

  1. We offer controls to set the margins, but they have no effect. There is no feedback, and it's unclear why this is happening.
  2. If the page specifies @media print { @page { margin: 0; } } and the page has the power to override margins, it can cause a situation where content is on the unwriteable area.

Chrome allows to override this setting, and I think this is the right thing to do.

Summary: print margin settings has no effect on pinterest.com and other sites → Popular sites prevent the print preview Margins menu from working (users need a way to override @page margins)

So basically we need to add a "Custom" menu item to the "Margins" options.

Whiteboard: [print2020_v81][old-ui-] → [print2020_v82][old-ui-]

(In reply to Jonathan Watt [:jwatt] from comment #6)

So basically we need to add a "Custom" menu item to the "Margins" options.

Probably this is a dup of bug 1664570.

Nope. Not a dup of bug 1664570. I just tested the latest patch on that bug, and the custom margins set in the UI are also ignored and do not override the @page margins. I guess not really surprising, on reflection.

Maybe what we need to do is define the following from the frontend UI:

  • Default - means use the @page style, or else the Firefox default margins (TBD)
  • Minimum - means the unwritable margins
  • None - means "trust me, the margins are in the content already, I know what I'm doing"
  • Custom - means use these custom margins

We would then add a flag to nsIPrintSettings to indicate which of the above four was selected, and nsPageFrame.cpp would be changed to only apply the @page style if 'Default' was selected in the UI.

The above would also imply that the change landed for bug 1665001 should be tweaked to allow 'None' to set the margins to zero despite the unwritable margin values.

Mark, Emilio, what are your thoughts on the above?

Flags: needinfo?(mstriemer)
Flags: needinfo?(emilio)

(In reply to Jonathan Watt [:jwatt] from comment #8)

Nope. Not a dup of bug 1664570. I just tested the latest patch on that bug, and the custom margins set in the UI are also ignored and do not override the @page margins. I guess not really surprising, on reflection.

Maybe what we need to do is define the following from the frontend UI:

  • Default - means use the @page style, or else the Firefox default margins (TBD)
  • Minimum - means the unwritable margins
  • None - means "trust me, the margins are in the content already, I know what I'm doing"
  • Custom - means use these custom margins

What about just a (default-checked) checkbox that says something like "allow the page to override this selection"? Our @page rule support is pretty basic atm, but you can in theory select individual pages and change the margins, and I can see how setting a custom margin but allow the page to override it for one or two pages is useful.

If so, I think we should just add a honorPageRuleMargins flag. Even without it, it seems with that and the existing settings we should be able to achieve the various behavior, except for the "none" case (which is a separate thing). If that seems fine, it should be trivial to add.

The above would also imply that the change landed for bug 1665001 should be tweaked to allow 'None' to set the margins to zero despite the unwritable margin values.

This seems like a potentially separate enhancement, I think we should probably do it in a separate bug. But sure, seems potentially useful I guess? Part of the issue is that I think on Windows we actually use the unwriteable margins to actually size the DT. But I may be misremembering, Bob may have changed that recently.

Flags: needinfo?(emilio)

Bug 1668406 implements a honorPageRuleMargins boolean attribute that we can expose however we want in the UI and is trivial.

Mark, I'm moving this to Toolkit now that Emilio has implemented nsIPrintSettings.honorPageRuleMargins in bug 1668406. See comment 8 and comment 9 on this bug for some suggestions on behaviour.

Component: Printing: Setup → Printing
Product: Core → Toolkit

Default definitely seems like the easiest solution, it does remove the .5" option which a user might want though. If we have some way of knowing if the page is setting the margins with @page then we could potentially add another option in this case "Set by page".

We might want to know if the page is setting the margins with @page anyway, and start with margins set to Default in that case. If you normally use margins:None but the page sets a reasonable default perhaps we should honour that.

Flags: needinfo?(mstriemer)
Whiteboard: [print2020_v82][old-ui-] → [print2020_v83][old-ui-]
Whiteboard: [print2020_v83][old-ui-] → [print2020_v84][old-ui-]
Whiteboard: [print2020_v84][old-ui-] → [print2020_v85][old-ui-]

(Moving bugs to 86, part 1.)

Whiteboard: [print2020_v85][old-ui-] → [print2020_v86][old-ui-]

Moving things to 88, cause we're mostly on Proton these days…

Whiteboard: [print2020_v86][old-ui-] → [print2020_v88] [old-ui-]

Actually, what I would prefer is for the @page margins and the FF margins to be additive.

I.e. If the page specifies top margin to 2 cm, and I set the top margin to 1 cm, the applied actual margin should be 3 cm.

With this approach the margin I set in my printer setting will reflect where on the page I want the content to be rendered, i.e. a bounding box, while the @page rules would define where within that box the content will be rendered.

If you're happy with the @page rules, simply set your own printer setting to "none".

In addition, since ridiculously many pages seem to have totally ridiculous @page rules, there should be an option to ignore them. If a web author f***s up the @page rules, it shouldn't result in me being unable to get a reasonable print.

If I'm the only person who finds the additivity suggestion good, I can accept ability to simply override @page rules. Solves the problem I guess....

Whiteboard: [print2020_v88] [old-ui-] → [print2020] [old-ui-]

is this something we would still consider S2 ?

Flags: needinfo?(mstriemer)
Flags: needinfo?(jwatt)

This doesn't really seem like an S2 to me, there isn't a workaround but theoretically there's a reason the margins are set by the webpage author.

That being said it would definitely be good to get this fixed. Looks like a one line change to get Default to respect the page's rule and the other's to respect the user's choice. Will write up a test and put up a patch

Assignee: nobody → mstriemer
Severity: S2 → S3
Flags: needinfo?(mstriemer)
Pushed by mstriemer@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/df3d9ecc4e03 Allow overriding @page margin rules r=sfoster
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 96 Branch
QA Whiteboard: [qa-96b-p2]
Flags: needinfo?(jwatt)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: