Bug 1747997 Comment 11 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

I tested this in an old build just before bug 1666247's patches, to see why this "regressed" there.

It looks like the relevant difference is: before bug 1666247, when `SetupToPrintContent` calls `GetDisplayTitleAndURL` for print-preview, it finds that `aSettings->GetDocURL()` is empty; and that causes `GetDisplayTitleAndURL` to fall back to the branded name for its `aTitle` outparam, even though it was passed the fallback-to-URL enum `eDocURLElseFallback`.  (Ironically, `GetDisplayTitleAndURL` then just proceeds to figure out the URL for itself, to populate its `aURLStr` outparam; it doesn't use that dynamically-determined page-URL as the fallback title.)

Whereas *after* bug 1666247, `aSettings->GetDocURL()` is **nonempty** -- it's got the document URL already, presumably provided by JS via one of the changes in bug 1666247 -- and that causes `GetDisplayTitleAndURL` to be able to fall back to that as the page-title (when given `eDocURLElseFallback`).

So, two takeaways:
(1) It seems odd/cobwebby that we have these two different fallback modes, and we use one for print-preview (and for early stages of actual printing), vs. the other for the final stage of actual-printing.  Perhaps we should drop `eDocURLElseFallback`, and/or make our usage consistent between the two callsites?

(2) If needed/simpler, we could also probably massage the JS to avoid providing the page's URL to the `aSettings` object, to restore our old behavior without needing to mess with the C++ here.

[I'm curious for jwatt's thoughts on both of these points; leaving ni open for his feedback on these when he's around.]
I tested this in an old build just before bug 1666247's patches, to see why this "regressed" there.

It looks like the relevant difference is: before bug 1666247, when `SetupToPrintContent` calls `GetDisplayTitleAndURL` for print-preview, it finds that `aSettings->GetDocURL()` is empty; and [that causes](https://searchfox.org/mozilla-central/rev/855814f769ac55bbb6a00e2170a6a876ab4cff3a/layout/printing/nsPrintJob.cpp#889,895) `GetDisplayTitleAndURL` to fall back to the branded name for its `aTitle` outparam, even though it was passed the fallback-to-URL enum `eDocURLElseFallback`.  (Ironically, `GetDisplayTitleAndURL` then just proceeds to figure out the URL for itself, to populate its `aURLStr` outparam; it doesn't use that dynamically-determined page-URL as the fallback title.)

Whereas *after* bug 1666247, `aSettings->GetDocURL()` is **nonempty** -- it's got the document URL already, presumably provided by JS via one of the changes in bug 1666247 -- and that causes `GetDisplayTitleAndURL` to be able to fall back to that as the page-title (when given `eDocURLElseFallback`).

So, two takeaways:
(1) It seems odd/cobwebby that we have these two different fallback modes, and we use one for print-preview (and for early stages of actual printing), vs. the other for the final stage of actual-printing.  Perhaps we should drop `eDocURLElseFallback`, and/or make our usage consistent between the two callsites?

(2) If needed/simpler, we could also probably massage the JS to avoid providing the page's URL to the `aSettings` object, to restore our old behavior without needing to mess with the C++ here.

[I'm curious for jwatt's thoughts on both of these points; leaving ni open for his feedback on these when he's around.]
I tested this in an old build just before bug 1666247's patches, to see why this "regressed" there.

It looks like the relevant difference is: before bug 1666247, when `SetupToPrintContent` calls `GetDisplayTitleAndURL` for print-preview, it finds that `aSettings->GetDocURL()` is empty; and [that causes](https://searchfox.org/mozilla-central/rev/855814f769ac55bbb6a00e2170a6a876ab4cff3a/layout/printing/nsPrintJob.cpp#889,895) `GetDisplayTitleAndURL` to fall back to the branded name for its `aTitle` outparam, even though it was passed the fallback-to-URL enum `eDocURLElseFallback`.  (Ironically, `GetDisplayTitleAndURL` then [just proceeds to figure out the URL for itself](https://searchfox.org/mozilla-central/rev/855814f769ac55bbb6a00e2170a6a876ab4cff3a/layout/printing/nsPrintJob.cpp#917,935), to populate its `aURLStr` outparam; it doesn't use that dynamically-determined page-URL as the fallback title.)

Whereas *after* bug 1666247, `aSettings->GetDocURL()` is **nonempty** -- it's got the document URL already, presumably provided by JS via one of the changes in bug 1666247 -- and that causes `GetDisplayTitleAndURL` to be able to fall back to that as the page-title (when given `eDocURLElseFallback`).

So, two takeaways:
(1) It seems odd/cobwebby that we have these two different fallback modes, and we use one for print-preview (and for early stages of actual printing), vs. the other for the final stage of actual-printing.  Perhaps we should drop `eDocURLElseFallback`, and/or make our usage consistent between the two callsites?

(2) If needed/simpler, we could also probably massage the JS to avoid providing the page's URL to the `aSettings` object, to restore our old behavior without needing to mess with the C++ here.

[I'm curious for jwatt's thoughts on both of these points; leaving ni open for his feedback on these when he's around.]
I tested this in an old build just before bug 1666247's patches, to see why this "regressed" there.

It looks like the relevant difference is: before bug 1666247, when `SetupToPrintContent` calls `GetDisplayTitleAndURL` for print-preview, it finds that `aSettings->GetDocURL()` is empty; and [that causes](https://searchfox.org/mozilla-central/rev/855814f769ac55bbb6a00e2170a6a876ab4cff3a/layout/printing/nsPrintJob.cpp#889,895) `GetDisplayTitleAndURL` to fall back to the branded name for its `aTitle` outparam, even though it was passed the fallback-to-URL enum `eDocURLElseFallback`.  (Ironically, `GetDisplayTitleAndURL` then [just proceeds to figure out the URL for itself](https://searchfox.org/mozilla-central/rev/855814f769ac55bbb6a00e2170a6a876ab4cff3a/layout/printing/nsPrintJob.cpp#917,935), to populate its `aURLStr` outparam; it doesn't use that dynamically-determined page-URL as the fallback title.)

Whereas *after* bug 1666247, `aSettings->GetDocURL()` is **nonempty** -- it's got the document URL already, presumably provided by JS via one of the changes in bug 1666247 -- and that causes `GetDisplayTitleAndURL` to be able to fall back to that as the page-title (when given `eDocURLElseFallback`).

So, tl;dr: bug 1666247 probably caused this visible behavior-change by virtue of helpfully providing the page URI in the settings object, which inadvertently exposed the fact that our missing-page-title fallback behavior is somewhat dependent on the presence/absence of that URI, specifically in the codepath that we take for print preview.

So, two takeaways:
(1) It seems odd/cobwebby that we have these two different fallback modes, and we use one for print-preview (and for early stages of actual printing), vs. the other for the final stage of actual-printing.  Perhaps we should drop `eDocURLElseFallback`, and/or make our usage consistent between the two callsites?

(2) If needed/simpler, we could also probably massage the JS to avoid providing the page's URL to the `aSettings` object, to restore our old behavior without needing to mess with the C++ here.

[I'm curious for jwatt's thoughts on both of these points; leaving ni open for his feedback on these when he's around.]
I tested this in an old build just before bug 1666247's patches, to see why this "regressed" there.

It looks like the relevant difference is: before bug 1666247, when `SetupToPrintContent` calls `GetDisplayTitleAndURL` for print-preview, it finds that `aSettings->GetDocURL()` is empty; and [that causes](https://searchfox.org/mozilla-central/rev/855814f769ac55bbb6a00e2170a6a876ab4cff3a/layout/printing/nsPrintJob.cpp#889,895) `GetDisplayTitleAndURL` to fall back to the branded name for its `aTitle` outparam, even though it was passed the fallback-to-URL enum `eDocURLElseFallback`.  (Ironically, `GetDisplayTitleAndURL` then [just proceeds to figure out the URL for itself](https://searchfox.org/mozilla-central/rev/855814f769ac55bbb6a00e2170a6a876ab4cff3a/layout/printing/nsPrintJob.cpp#917,935), to populate its `aURLStr` outparam; it doesn't use that dynamically-determined page-URL as the fallback title.)

Whereas *after* bug 1666247, `aSettings->GetDocURL()` is **nonempty** -- it's got the document URL already, presumably provided by JS via one of the changes in bug 1666247 -- and that causes `GetDisplayTitleAndURL` to be able to fall back to that as the page-title (when given `eDocURLElseFallback`).

So, tl;dr: bug 1666247 probably caused this visible behavior-change by virtue of helpfully providing the page URI in the settings object, which inadvertently exposed the fact that our missing-page-title fallback behavior is somewhat dependent on the presence/absence of that URI, specifically in the codepath that we take for print preview.

So, two takeaways:
(1) It seems odd/cobwebby that we have these two different fallback modes, and we use one for print-preview (and for early stages of actual printing), vs. the other for the final stage of actual-printing.  Perhaps we should drop `eDocURLElseFallback`, and/or make our usage consistent between the two callsites?

(2) If needed/simpler, we could also probably massage the JS to avoid providing the page's URL to the `aSettings` object, to restore our old behavior without needing to mess with the C++ here. (But ideally it would be good to have a coherent story around how the underlying codepaths behave, too.)

[I'm curious for jwatt's thoughts on both of these points; leaving ni open for his feedback on these when he's around.]

Back to Bug 1747997 Comment 11