1411121 - Remove the printdialog.xul and related files.

Status: RESOLVED FIXED

(Toolkit :: Printing, enhancement, P3)

Description

[Mantaroh Yoshinaga[:mantaroh]]

+++ This bug was initially created as a clone of Bug #1409972 +++

We can remove the printing xul dialog(i.e. printdialog.xul) from printingui/unixshared/nsPrintingPromptService.cpp since this code is unreachable now.
This code introduced bug 193001. In this era, gecko support BeOS and QNX, this platform uses embedding/components/printingui/src/unixshared/nsPrintingPromptService.cpp[1] and these doesn't have implementation of nsIPrintDialogSerice[2].

[1] https://github.com/mozilla/gecko-dev/blob/9e6a8820f7964ed36d63acf13b84b0cde12caaa6/embedding/components/printingui/src/Makefile.in#L46
[2] https://github.com/mozilla/gecko-dev/tree/9e6a8820f7964ed36d63acf13b84b0cde12caaa6/widget/src

Comment 1

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: Bug 1411121 - Part 1: Return NS_ERROR_FAILURE if nsIPrintDialogService doesn't exist on GTK when showing print dialog.

A printingui uses the xul print dialog(i.e. printdialog.xul) if there
are not nsIPrintDialogService. But this code is unreachable code since
current gtk widget has nsIPrintDialogService always.
So this patch will remove this code.

Review commit: https://reviewboard.mozilla.org/r/192320/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/192320/

Comment 2

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: Bug 1411121 - Part 2: Remove printdialog.xul and printjoboptions.xul.

This patch will remove printing xul dialog and related files since a caller of
this xul dialog doesn't exist.
The printjoboptions.xul is only used by printdialog.xul.

Review commit: https://reviewboard.mozilla.org/r/192322/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/192322/

Comment 3

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: Bug 1411121 - Part 3: Remove the nsIPrintingPromptService::showPrinterProperties()

We can remove nsIPrintingPromptService::showPrinterProperties() since this
caller doesn't exist and this implementation of each platform returns
NS_ERROR_NOT_IMPLEMENTED.

Review commit: https://reviewboard.mozilla.org/r/192332/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/192332/

Comment 4

[Mantaroh Yoshinaga[:mantaroh]]

Try build:
 https://treeherder.mozilla.org/#/jobs?repo=try&revision=664000df97297ad8188cd1b29d76da6dd083a3ea

Comment 5

[Karl Tomlinson (:karlt)]

Comment on attachment 8921331 [details]
Bug 1411121 - Part 1: Return NS_ERROR_FAILURE if nsIPrintDialogService doesn't exist on GTK when showing print dialog.

https://reviewboard.mozilla.org/r/192320/#review197922

::: commit-message-b6ba3:4
(Diff revision 1)
> +are not nsIPrintDialogService. But this code is unreachable code since
> +current gtk widget has nsIPrintDialogService always.

What prevents this code from being called in the Android port?

Comment 6

[Mantaroh Yoshinaga[:mantaroh]]

Comment on attachment 8921331 [details]
Bug 1411121 - Part 1: Return NS_ERROR_FAILURE if nsIPrintDialogService doesn't exist on GTK when showing print dialog.

https://reviewboard.mozilla.org/r/192320/#review197978

::: commit-message-b6ba3:4
(Diff revision 1)
> +are not nsIPrintDialogService. But this code is unreachable code since
> +current gtk widget has nsIPrintDialogService always.

Thank you so much for review this.

I think that Android platform will not call this nsIPrintingPromptService::ShowPrintDialog/ShowPageSetup. Because android calls silent print always.[1][2]. If silent print is enable, print engine will not call this nsIPrintingPromptService::ShowDialog on layout[3].

[1] http://searchfox.org/mozilla-central/rev/1285ae3e810e2dbf2652c49ee539e3199fcfe820/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java#3922,3928
[2] http://searchfox.org/mozilla-central/rev/1285ae3e810e2dbf2652c49ee539e3199fcfe820/mobile/android/chrome/content/PrintHelper.js#31
[3] http://searchfox.org/mozilla-central/rev/1285ae3e810e2dbf2652c49ee539e3199fcfe820/layout/printing/nsPrintEngine.cpp#600,617

Comment 7

[Karl Tomlinson (:karlt)]

Comment on attachment 8921331 [details]
Bug 1411121 - Part 1: Return NS_ERROR_FAILURE if nsIPrintDialogService doesn't exist on GTK when showing print dialog.

https://reviewboard.mozilla.org/r/192320/#review198450

Thanks!

Comment 8

[Mantaroh Yoshinaga[:mantaroh]]

(In reply to Karl Tomlinson (:karlt) from comment #7)
> Comment on attachment 8921331 [details]
> Bug 1411121 - Part 1: Return NS_ERROR_FAILURE if nsIPrintDialogService
> doesn't exist on GTK when showing print dialog.
> 
> https://reviewboard.mozilla.org/r/192320/#review198450
> 
> Thanks!

Thank you for confirming this!

Comment 9

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8921332 [details]
Bug 1411121 - Part 2: Remove printdialog.xul and printjoboptions.xul.

https://reviewboard.mozilla.org/r/192322/#review200198

Great, thanks!

Comment 10

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8921333 [details]
Bug 1411121 - Part 3: Remove the nsIPrintingPromptService::showPrinterProperties()

https://reviewboard.mozilla.org/r/192332/#review200200

Thanks for the clean-up!

Comment 11

[Mantaroh Yoshinaga[:mantaroh]]

Thank you so much!

Comment 12

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s dc18133ffbc3 -d eb4f3467df5c: rebasing 431361:dc18133ffbc3 "Bug 1411121 - Part 1: Return NS_ERROR_FAILURE if nsIPrintDialogService doesn't exist on GTK when showing print dialog. r=karlt"
merging toolkit/components/printingui/unixshared/nsPrintingPromptService.cpp
warning: conflicts while merging toolkit/components/printingui/unixshared/nsPrintingPromptService.cpp! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 13

[Mantaroh Yoshinaga[:mantaroh]]

Comment on attachment 8921331 [details]
Bug 1411121 - Part 1: Return NS_ERROR_FAILURE if nsIPrintDialogService doesn't exist on GTK when showing print dialog.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/192320/diff/1-2/

Comment 14

[Mantaroh Yoshinaga[:mantaroh]]

Comment on attachment 8921332 [details]
Bug 1411121 - Part 2: Remove printdialog.xul and printjoboptions.xul.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/192322/diff/1-2/

Comment 15

[Mantaroh Yoshinaga[:mantaroh]]

Comment on attachment 8921333 [details]
Bug 1411121 - Part 3: Remove the nsIPrintingPromptService::showPrinterProperties()

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/192332/diff/1-2/

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d390f8888d71
Part 1: Return NS_ERROR_FAILURE if nsIPrintDialogService doesn't exist on GTK when showing print dialog. r=karlt
https://hg.mozilla.org/integration/autoland/rev/9d8722dd9693
Part 2: Remove printdialog.xul and printjoboptions.xul. r=mconley
https://hg.mozilla.org/integration/autoland/rev/9f02fcb3d28c
Part 3: Remove the nsIPrintingPromptService::showPrinterProperties() r=mconley

Comment 17

[Natalia Csoregi [:nataliaCs]]

Backed out for failing browser-chrome browser/base/content/test/static/browser_all_files_referenced.js 

Failure log : https://treeherder.mozilla.org/logviewer.html#?job_id=141267730&repo=autoland&lineNumber=2059

https://hg.mozilla.org/integration/autoland/rev/400a2d21fb701dbcf0977b51fa81345e51f3ddca

Push that caused the failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=9f02fcb3d28cabff600c933f3f436c0ed1c6a380&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception

Comment 18

[Mantaroh Yoshinaga[:mantaroh]]

Comment on attachment 8921331 [details]
Bug 1411121 - Part 1: Return NS_ERROR_FAILURE if nsIPrintDialogService doesn't exist on GTK when showing print dialog.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/192320/diff/2-3/

Comment 19

[Mantaroh Yoshinaga[:mantaroh]]

Comment on attachment 8921332 [details]
Bug 1411121 - Part 2: Remove printdialog.xul and printjoboptions.xul.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/192322/diff/2-3/

Comment 20

[Mantaroh Yoshinaga[:mantaroh]]

Comment on attachment 8921333 [details]
Bug 1411121 - Part 3: Remove the nsIPrintingPromptService::showPrinterProperties()

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/192332/diff/2-3/

Comment 21

[Mantaroh Yoshinaga[:mantaroh]]

https://reviewboard.mozilla.org/r/192318/diff/2-3/

This test checks that package contain the correct resources, So I should remove the printdialog.dtd and printjoboptions.dtd from this test.

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5b18702a1f1380187bbb60848e131d7bf2148546

Comment 22

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: Bug 1411121 - Part 4. Remove printPageSetup resources from linux package since Linux will display native dialog.

This patch will remove printPageSetup resources from Linux package. This xul is
'PageSetup' dialog, but gecko doesn't use this dialog since gecko will display
a native page setup dialog.
(For Detail, See nsPrintDialogServiceGTK::ShowPageSetup)

Review commit: https://reviewboard.mozilla.org/r/195716/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/195716/

Comment 23

[Mantaroh Yoshinaga[:mantaroh]]

Comment on attachment 8921331 [details]
Bug 1411121 - Part 1: Return NS_ERROR_FAILURE if nsIPrintDialogService doesn't exist on GTK when showing print dialog.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/192320/diff/3-4/

Comment 24

[Mantaroh Yoshinaga[:mantaroh]]

Comment on attachment 8921332 [details]
Bug 1411121 - Part 2: Remove printdialog.xul and printjoboptions.xul.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/192322/diff/3-4/

Comment 25

[Mantaroh Yoshinaga[:mantaroh]]

Comment on attachment 8921333 [details]
Bug 1411121 - Part 3: Remove the nsIPrintingPromptService::showPrinterProperties()

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/192332/diff/3-4/

Comment 26

[Mantaroh Yoshinaga[:mantaroh]]

(In reply to Mantaroh Yoshinaga[:mantaroh] from comment #21)
> https://reviewboard.mozilla.org/r/192318/diff/2-3/
> 
> This test checks that package contain the correct resources, So I should
> remove the printdialog.dtd and printjoboptions.dtd from this test.
> 
> Try:
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=5b18702a1f1380187bbb60848e131d7bf2148546

https://treeherder.mozilla.org/logviewer.html#?job_id=141518233&repo=try

Linux test is failed. I think that my patch will remove reference of printPageSetup.xul on Linux, as result, this test will fail.
So I need to remove printPageSetup's resources from linux package.

Try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=187044c21482f54f03efd835ff00803975462c5c

Mike,
I'm Sorry to bother you. Would you please confirm last patch?

Comment 27

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8924453 [details]
Bug 1411121 - Part 4. Remove printPageSetup resources from linux package since Linux will display native dialog.

https://reviewboard.mozilla.org/r/195716/#review201126

Looks good! Might be worth a try build though just to make sure we didn't miss anything. Thanks!

Comment 28

[Mantaroh Yoshinaga[:mantaroh]]

Comment on attachment 8921331 [details]
Bug 1411121 - Part 1: Return NS_ERROR_FAILURE if nsIPrintDialogService doesn't exist on GTK when showing print dialog.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/192320/diff/4-5/

Comment 29

[Mantaroh Yoshinaga[:mantaroh]]

Comment on attachment 8921332 [details]
Bug 1411121 - Part 2: Remove printdialog.xul and printjoboptions.xul.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/192322/diff/4-5/

Comment 30

[Mantaroh Yoshinaga[:mantaroh]]

Comment on attachment 8921333 [details]
Bug 1411121 - Part 3: Remove the nsIPrintingPromptService::showPrinterProperties()

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/192332/diff/4-5/

Comment 31

[Mantaroh Yoshinaga[:mantaroh]]

Comment on attachment 8924453 [details]
Bug 1411121 - Part 4. Remove printPageSetup resources from linux package since Linux will display native dialog.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/195716/diff/1-2/

Comment 32

[Mantaroh Yoshinaga[:mantaroh]]

(In reply to Mantaroh Yoshinaga[:mantaroh] from comment #31)
> Comment on attachment 8924453 [details]
> Bug 1411121 - Part 4. Remove printPageSetup resources from linux package
> since Linux will display native dialog.
> 
> Review request updated; see interdiff:
> https://reviewboard.mozilla.org/r/195716/diff/1-2/

Rebased it.

Try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ccdadaab42f1bc1851d105a597b7a995d420adcb

Comment 33

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b503682a7c6e
Part 1: Return NS_ERROR_FAILURE if nsIPrintDialogService doesn't exist on GTK when showing print dialog. r=karlt
https://hg.mozilla.org/integration/autoland/rev/1ca360e19880
Part 2: Remove printdialog.xul and printjoboptions.xul. r=mconley
https://hg.mozilla.org/integration/autoland/rev/db6983d22fd3
Part 3: Remove the nsIPrintingPromptService::showPrinterProperties() r=mconley
https://hg.mozilla.org/integration/autoland/rev/110873e70443
Part 4. Remove printPageSetup resources from linux package since Linux will display native dialog. r=mconley

Comment 34

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/b503682a7c6e
https://hg.mozilla.org/mozilla-central/rev/1ca360e19880
https://hg.mozilla.org/mozilla-central/rev/db6983d22fd3
https://hg.mozilla.org/mozilla-central/rev/110873e70443