Closed Bug 1674106 Opened 3 months ago Closed 2 months ago

Print modal turns inaccessible when toggling between paper size using static custom values

Categories

(Toolkit :: Printing, defect, P1)

defect

Tracking

()

VERIFIED FIXED
85 Branch
Tracking Status
firefox83 --- wontfix
firefox84 --- verified
firefox85 --- verified

People

(Reporter: Anca, Assigned: emalysz)

References

(Blocks 2 open bugs)

Details

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

Attachments

(5 files)

Attached image screencast issue.gif

Affected versions

  • 83.0b5
  • 84.0a1 (2020-10-28)

Affected platforms

  • Windows 10
  • Windows 7
  • macOS 10.15
  • Ubuntu 20.04

Steps to reproduce

  1. Launch Firefox
  2. Make sure print.tab_modal.enabled is set on true
  3. Hit CTRL + P on any page (eg. https://en.wikipedia.org/wiki/Facebook))
  4. Set paper size to A4
  5. Set valid custom value
  6. Select another paper size option that turns the custom values invalid
  7. Set paper size back to A4
    .
    Expected result
  • All the elements inside the modal can be accessed and they are functional

Actual result

  • Most of the element inside the Print modal are inaccessible

Regression range

  • Not a regression, present since custom margins implementation

Additional notes

  • The modal is once again active if it is closed and reopened again

Suggested severity

  • S2
Whiteboard: [print2020_v84] [old-ui+] → [print2020_v84] [old-ui-]
Attached image UnresponsiveTab.gif

The symptoms seem to get worse if reproducing this issue and exiting the print preview while the print preview is in a loading state. The tab is no longer responding to actions like (tab refresh button, back navigation button,etc).
Note: This behavior seems to be encountered with both Fission enabled & disabled.

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

After discussion, we're going to try to get this fixed in 84…

Severity: -- → S2
Priority: -- → P1
Whiteboard: [print2020_v85] [old-ui-] → [print2020_v84] [old-ui-]

Custom margins should reset to default values if there is an invalid value detected.

The problem is that we are not taking into account the unwriteable margins in getSettingsToUpdate, but we are when setting the maxHeight and maxWidth within the element. These values are async properties and are resolved within fetchPaperMargins. I think the best route forward would be to move those calculations directly into updateSettings underneath this call (https://searchfox.org/mozilla-central/rev/16d30bafd4e5276d6d3c632fb52a6c71e739cc44/toolkit/components/printing/content/print.js#544-546), return the paperWrapper, and then modify changedSettings if necessary.

I also had made a typo here: https://searchfox.org/mozilla-central/rev/16d30bafd4e5276d6d3c632fb52a6c71e739cc44/toolkit/components/printing/content/print.js#477. That should read customMarginBottom.

Assignee: nobody → emalysz
Status: NEW → ASSIGNED
Pushed by emalysz@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/58572979c431
validate custom margin values when paper size changes and change if necessary r=sfoster
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → 84 Branch

The patch landed in nightly and beta is affected.
:emalysz, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(emalysz)
Flags: needinfo?(emalysz)
Flags: qe-verify+

I can still reproduce the issue for some of the paper size on latest Nightly across platforms

Flags: needinfo?(emalysz)
Status: RESOLVED → REOPENED
Flags: needinfo?(emalysz)
Resolution: FIXED → ---
Whiteboard: [print2020_v84] [old-ui-] → [print2020_v85] [old-ui-]
Attachment #9190369 - Attachment description: Bug 1674106, reset userChangedSettings when updating an invalid margin after a paper size switch → Bug 1674106, delete custom margins from userChangedSettings when updating an invalid margin after a paper size switch
Pushed by emalysz@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/36f51561645e
delete custom margins from userChangedSettings when updating an invalid margin after a paper size switch r=mstriemer
Status: REOPENED → RESOLVED
Closed: 3 months ago2 months ago
Resolution: --- → FIXED
Target Milestone: 84 Branch → 85 Branch

Comment on attachment 9190369 [details]
Bug 1674106, delete custom margins from userChangedSettings when updating an invalid margin after a paper size switch

Beta/Release Uplift Approval Request

  • User impact if declined: We can lock the print UI for a user with custom margins that switches between paper sizes multiple times.

This is an edge case, but still is an issue that we should address

  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: 1. Open the new print preview dialog
  1. Set paper size to us letter
  2. Set right/left margins to 4 each
  3. Switch to A5 paper sizes. Notice margins reset to 0.5
  4. Switch to A6 paper size. Notice margins are still 0.5 and the UI is usable
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): no strings, just two lines of code that address an edge case
  • String changes made/needed: n/a
Attachment #9190369 - Flags: approval-mozilla-beta?

Comment on attachment 9185337 [details]
Bug 1674106: validate custom margin values when paper size changes and change if necessary

Sorry, I believe the first patch would not require beta uplift, as it should have made the cutoff for 84

Attachment #9185337 - Flags: approval-mozilla-beta?
Attachment #9185337 - Flags: approval-mozilla-beta?
QA Whiteboard: [qa-triaged]

Comment on attachment 9190369 [details]
Bug 1674106, delete custom margins from userChangedSettings when updating an invalid margin after a paper size switch

Thanks for including automated tests. Approved for 84.0b8.

Attachment #9190369 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Duplicate of this bug: 1679386

Verified fixed with Fx 84.0b8 and Fx 85.0a1(2020-12-03) on Windows 10, macOS 10.15 and Ubuntu 18.04.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.