Closed Bug 1663173 Opened 3 months ago Closed 3 months ago

New Print modal is broken inside Customize page

Categories

(Firefox :: Toolbars and Customization, defect, P3)

defect

Tracking

()

VERIFIED FIXED
82 Branch
Tracking Status
firefox-esr68 --- unaffected
firefox-esr78 --- unaffected
firefox80 --- unaffected
firefox81 --- wontfix
firefox82 --- verified

People

(Reporter: Anca, Assigned: Gijs)

References

(Blocks 2 open bugs)

Details

(Keywords: regression, Whiteboard: [print2020_v82])

Attachments

(2 files)

Affected versions

  • 81.0b6
  • 82.0a1 (2020-09-04)

Affected platforms

  • Windows 10
  • macOS 10.15
  • Ubuntu 18.04

Steps to reproduce

  1. Launch Fx
  2. Make sure print.tab_modal.enabled is set on true
  3. Open customize page
  4. Hit Ctrl + P

Expected result

  • Print modal is broken and no interaction with the modal can be made)

Actual result

  • Print modal is not broken

Regression range

Suggested severity

  • S3

(Might be a more general TabDialogBox bug, so tagging Gijs…)

Whiteboard: [print2020_v81]

Print is supposed to be disabled in customize mode (and indeed, the menuitem is disabled, so it's curious that the shortcut works...). Other dialogs also won't / shouldn't appear in customize mode. The reason things get mispositioned is that customize mode displays instead of a "real" tab.

Assignee: nobody → gijskruitbosch+bugs
Severity: -- → S3
Component: Printing → Toolbars and Customization
Priority: -- → P3
Product: Toolkit → Firefox
Has Regression Range: --- → yes
Has STR: --- → yes
Status: NEW → ASSIGNED
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/ffe1c9f9f9df
disable commands that don't need to be enabled in customize mode, r=mconley
Whiteboard: [print2020_v81] → [print2020_v82]

(In reply to Razvan Maries from comment #5)

Backed out for perma failures.

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedTaskRun=Bp9-_jTkRIyLnKME0Y3cgw.0&resultStatus=testfailed%2Cbusted%2Cexception&revision=8f7d601341b453b9ec17d45922e9988fa83a6f0f&searchStr=bc&failure_classification_id=2

Log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=315253986&repo=autoland&lineNumber=11331

Backout: https://hg.mozilla.org/integration/autoland/rev/3a2d17ccc986413be16be59e04c0e4a6fe51a638

Bah, this is dumb. Here's why tests broke:

  1. we enter customize mode. Commands get disabled
  2. we construct widgets to show them in the palette. They have "command" attributes. They inherit the "disabled" command from the command in question
  3. we leave customize mode. The widgets from (2) go back into the palette
  4. we remove the disabled attributes from the commands. Now, first off, I'm not sure how well this propagates even to things that are in the DOM at that point (cf. bug 309953). But the commands that went back into the palette definitely don't get those attributes re-sync'd.
  5. we try to use some of those widgets by adding them into the UI using API calls. They are still disabled because we missed them in step (4).

It's not super clear to me if this can happen in normal use, as users can't really manipulate the toolbar using API calls, and if items get moved into a toolbar / menu during customize mode, they won't be missed in step (4). Still, it feels like a bad pitfall to leave lying around, and it points to maybe the location where the enabling/disabling happens not being the right one, or something. So I'll look if I can improve the patch to avoid this breaking.

Flags: needinfo?(gijskruitbosch+bugs)
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/77840a2500c4
disable commands that don't need to be enabled in customize mode, r=mconley
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → 82 Branch
Flags: qe-verify+

Reproduced the initial issue using old Nightly from (2020-09-04), verified that the issue is fixed in Firefox 82.0b2 across platforms (Windows 10 64bit, macOS 10.15 and Ubuntu 18.04).

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