Closed Bug 1658299 Opened 4 years ago Closed 4 years ago

Implement a Promise returning nsIPrinter.createDefaultSettings()

Categories

(Core :: Printing: Setup, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
81 Branch
Tracking Status
firefox81 --- fixed

People

(Reporter: jwatt, Assigned: nordzilla)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [print2020_v81])

Attachments

(2 files, 1 obsolete file)

nsPrinterListWin::InitPrintSettingsFromPrinter makes a blocking call to system APIs, which unfortunately is unnecessarily slowing down the new printing UI since that is called under this frontend code. The frontend code should not need to get these defaults last-used settings for that printer have been saved to prefs and the frontend code will be using those values.

We could improve things by implementing a new Promise returning nsIPrinter.defaultSettings attribute, and get rid of nsIPrintSettingsService.initPrintSettingsFromPrinter altogether.

Although I guess the object should should be mutable, so a creation function would be better.

Summary: Implement a Promise returning nsIPrinter.defaultSettings → Implement a Promise returning nsIPrinter.createDefaultSettings()
Blocks: 1651112
No longer blocks: 1631440
Assignee: nobody → enordin
Status: NEW → ASSIGNED
Attachment #9170110 - Attachment description: Bug 1658299 - WIP: Add createDefaultSettings() function to nsIPrinter IDL → Bug 1658299 - Part 1: Add createDefaultSettings() to nsIPrinter for CUPS WIP
Attachment #9170110 - Attachment description: Bug 1658299 - Part 1: Add createDefaultSettings() to nsIPrinter for CUPS WIP → Bug 1658299 - Part 1: Add createDefaultSettings() to nsIPrinter for CUPS r=jwatt,alaskanemily,emilio

This should really be P1. Without this the new UI will create a print preview with zero margins (at least on macOS and Linux), which will in most cases result in cropping of the print output, which would really annoy users.

Priority: P2 → P1

This retrieves the same settings from the printer as
InitPrintSettingsFromPrinter.
There is probably a lot of scope for de-duplication of code here, but I decided
to try and protect the old fallback paths.

This retrieves the same settings from the printer as
InitPrintSettingsFromPrinter.
There is probably a lot of scope for de-duplication of code here, but I decided
to try and protect the old fallback paths.

Phabricator Revision: https://phabricator.services.mozilla.com/D87604

Depends on D87125

Attachment #9171082 - Attachment is obsolete: true
Pushed by jwatt@jwatt.org: https://hg.mozilla.org/integration/autoland/rev/ebf8dfa204ea Part 1: Add createDefaultSettings() to nsIPrinter for CUPS r=jwatt,emilio,geckoview-reviewers,owlish

Backed out changeset ebf8dfa204ea (bug 1658299) for test_printer_default_settings.html failures.

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&selectedTaskRun=T8i79jNIQ2CbiaoWuMLD6w.0&fromchange=ebf8dfa204ea618c6e77ec6f06c4c7af786bee9b&searchStr=mochitest&tochange=cda275a5ba19aa62274f9d0264440216be89eaad

Backout link: https://hg.mozilla.org/integration/autoland/rev/cda275a5ba19aa62274f9d0264440216be89eaad

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=313511953&repo=autoland&lineNumber=8674

[task 2020-08-20T02:29:38.563Z] 02:29:38     INFO - TEST-START | layout/base/tests/chrome/test_printer_default_settings.html
[task 2020-08-20T02:29:38.602Z] 02:29:38     INFO - GECKO(1792) | [4280, Main Thread] WARNING: Failed to retarget HTML data delivery to the parser thread.: file /builds/worker/checkouts/gecko/parser/html/nsHtml5StreamParser.cpp, line 1131
[task 2020-08-20T02:29:38.681Z] 02:29:38     INFO - GECKO(1792) | [4280, Main Thread] WARNING: '!devmode', file /builds/worker/checkouts/gecko/widget/windows/nsPrintSettingsWin.cpp, line 183
[task 2020-08-20T02:29:38.681Z] 02:29:38     INFO - GECKO(1792) | console.log: "number"
[task 2020-08-20T02:29:38.700Z] 02:29:38     INFO - GECKO(1792) | [4280, Main Thread] WARNING: '!devmode', file /builds/worker/checkouts/gecko/widget/windows/nsPrintSettingsWin.cpp, line 183
[task 2020-08-20T02:29:38.701Z] 02:29:38     INFO - GECKO(1792) | console.log: "number"
[task 2020-08-20T02:29:38.701Z] 02:29:38     INFO - Buffered messages logged at 02:29:38
[task 2020-08-20T02:29:38.701Z] 02:29:38     INFO - Microsoft XPS Document Writer
[task 2020-08-20T02:29:38.701Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Printer name should be a string. 
[task 2020-08-20T02:29:38.702Z] 02:29:38     INFO - TEST-FAIL | layout/base/tests/chrome/test_printer_default_settings.html | Print settings' printer should match the printer that created them. - got "", expected "Microsoft XPS Document Writer"
[task 2020-08-20T02:29:38.702Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Paper name should never be null. 
[task 2020-08-20T02:29:38.702Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Paper width hould never be null. 
[task 2020-08-20T02:29:38.702Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Paper height hould never be null. 
[task 2020-08-20T02:29:38.703Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Paper margins should be greater than or equal to zero. 
[task 2020-08-20T02:29:38.703Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Paper margins should be greater than or equal to zero. 
[task 2020-08-20T02:29:38.703Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Paper margins should be greater than or equal to zero. 
[task 2020-08-20T02:29:38.703Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Paper margins should be greater than or equal to zero. 
[task 2020-08-20T02:29:38.704Z] 02:29:38     INFO - TEST-FAIL | layout/base/tests/chrome/test_printer_default_settings.html | Print settings' color mode should match the printer's color support. - got false, expected true
[task 2020-08-20T02:29:38.704Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Print settings were initialized from printer 
[task 2020-08-20T02:29:38.704Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | undefined assertion name 
[task 2020-08-20T02:29:38.704Z] 02:29:38     INFO - Fax
[task 2020-08-20T02:29:38.704Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Printer name should be a string. 
[task 2020-08-20T02:29:38.704Z] 02:29:38     INFO - TEST-FAIL | layout/base/tests/chrome/test_printer_default_settings.html | Print settings' printer should match the printer that created them. - got "", expected "Fax"
[task 2020-08-20T02:29:38.704Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Paper name should never be null. 
[task 2020-08-20T02:29:38.704Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Paper width hould never be null. 
[task 2020-08-20T02:29:38.705Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Paper height hould never be null. 
[task 2020-08-20T02:29:38.705Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Paper margins should be greater than or equal to zero. 
[task 2020-08-20T02:29:38.705Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Paper margins should be greater than or equal to zero. 
[task 2020-08-20T02:29:38.705Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Paper margins should be greater than or equal to zero. 
[task 2020-08-20T02:29:38.705Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Paper margins should be greater than or equal to zero. 
[task 2020-08-20T02:29:38.705Z] 02:29:38     INFO - Buffered messages finished
[task 2020-08-20T02:29:38.705Z] 02:29:38     INFO - TEST-UNEXPECTED-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Print settings' color mode should match the printer's color support. - false should equal false
[task 2020-08-20T02:29:38.705Z] 02:29:38     INFO - TEST-INFO | expected FAIL
[task 2020-08-20T02:29:38.705Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Print settings were initialized from printer 
[task 2020-08-20T02:29:38.706Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | undefined assertion name 
[task 2020-08-20T02:29:38.721Z] 02:29:38     INFO - GECKO(1792) | MEMORY STAT | vsize 913MB | vsizeMaxContiguous 650MB | residentFast 295MB | heapAllocated 117MB
Flags: needinfo?(enordin)

Gah, the guards I put on the test to ensure that those assertions didn't run on Windows only worked for Windows 10 and not Windows 7.

I'm just going to remove those assertions from the test in part 1, since part 2 adds that functionality is working just fine.

Here is a try run that includes all the platforms, including Windows 7:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=919e30b7ef0ce675c3035872c0ed7cd47b3d463e

Flags: needinfo?(enordin)
Pushed by abutkovits@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/35d913aaea4d Part 1: Add createDefaultSettings() to nsIPrinter for CUPS r=jwatt,emilio,geckoview-reviewers,owlish
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 81 Branch
Pushed by jwatt@jwatt.org: https://hg.mozilla.org/integration/autoland/rev/96c093dbd493 Part 2: Add createDefaultSettings() to nsIPrinter for Windows. r=jwatt,jfkthame
Depends on: 1660993
Blocks: 1660997
Depends on: 1661731
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: