Closed Bug 1054810 Opened 6 years ago Closed 6 years ago

Print selection is completely broken. The selected text is not printed at all.

Categories

(Core :: Layout: Text and Fonts, defect)

34 Branch
x86_64
Windows 7
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla34
Tracking Status
firefox33 --- unaffected
firefox34 - verified

People

(Reporter: alice0775, Assigned: smontagu)

References

(Depends on 1 open bug, )

Details

(Keywords: regression)

Attachments

(2 files)

[Tracking Requested - why for this release]:

[Tracking Requested - why for this release]:

Regression window(m-i)
Good:
https://hg.mozilla.org/integration/mozilla-inbound/rev/810551a5ee64
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:34.0) Gecko/20100101 Firefox/34.0 ID:20140724013012
Bad:
https://hg.mozilla.org/integration/mozilla-inbound/rev/634d33dc9d3e
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:34.0) Gecko/20100101 Firefox/34.0 ID:20140724013609
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=810551a5ee64&tochange=634d33dc9d3e

Regressed by: Bug 789096
Summary: Print selection is completely broken → Print selection is completely broken. The selected text is not printed at all.
In local build
Last Good: a4ba6995c87e
First Bad: 634d33dc9d3e
Regressed by: 
  634d33dc9d3e	Simon Montagu — Bug 789096 patch 10: make Reflow set nsHTMLReflowMetrics.ISize and BSize instead of Width and Height. r=jfkthame
smontagu, do you have cycles to take a look?
Flags: needinfo?(smontagu)
I can reproduce, with e.g. data:text/plain,Testing  (selecting some or all of the text, & doing Print w/ "print selection" selected, using the "Print to file" printer.  I'm using current Nightly and Ubuntu 14.04.
Assignee: nobody → smontagu
Flags: needinfo?(smontagu)
I can reproduce on OSX, and confirm that backing out bug 789096 patch 10 fixes the bug (after some initial confusion caused by bug 428111)
Try push at https://tbpl.mozilla.org/?tree=Try&rev=59e6a551c04b. I'll attach the patch and ask review after double-checking that I didn't make a similar mistake elsewhere.
Attached patch PatchSplinter Review
Fixes this bug, and a parallel issue in nsPageFrame.cpp, plus an outdated line that I accidentaly didn't remove in nsTableRowFrame.cpp
Attachment #8475083 - Flags: review?(jfkthame)
Comment on attachment 8475083 [details] [diff] [review]
Patch

Review of attachment 8475083 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/generic/nsPageContentFrame.cpp
@@ +101,5 @@
>    WritingMode wm = aReflowState.GetWritingMode();
> +  LogicalSize finalSize(wm, aReflowState.ComputedISize(),
> +                        aReflowState.ComputedBSize() == NS_UNCONSTRAINEDSIZE ?
> +                          aDesiredSize.BSize(wm) :
> +                          aReflowState.ComputedBSize());

Do we require that aReflowState and aDesiredSize share the same writing mode? If so, we can make this simpler and more readable by setting aDesiredSize directly:

  aDesiredSize.ISize(wm) = aReflowState.ComputedISize();
  if (aReflowState.ComputedBSize() != NS_UNCONSTRAINEDSIZE) {
    aDesiredSize.BSize(wm) = aReflowState.ComputedBSize();
  }

So no need for the finalSize variable at all here. Or is there something I'm not seeing?

(And if we don't require those writing modes to match, then the use of aDesiredSize.BSize(wm) here would be a problem.)

::: layout/generic/nsPageFrame.cpp
@@ +151,5 @@
>    WritingMode wm = aReflowState.GetWritingMode();
> +  LogicalSize finalSize(wm, aReflowState.AvailableISize(),
> +                        aReflowState.AvailableBSize() == NS_UNCONSTRAINEDSIZE ?
> +                          aDesiredSize.BSize(wm) :
> +                          aReflowState.AvailableBSize());

Ditto

::: layout/tables/nsTableRowFrame.cpp
@@ -943,5 @@
>            ascent = ((nsTableCellFrame *)kidFrame)->GetCellBaseline();
>          }
>          nscoord descent = desiredSize.BSize(rowWM) - ascent;
>          UpdateHeight(desiredSize.BSize(rowWM), ascent, descent, &aTableFrame, cellFrame);
> -        UpdateHeight(desiredSize.Height(), ascent, descent, &aTableFrame, cellFrame);

Oops! Clearly my eyes had glazed over by the time I was reviewing.... :(
Attached patch Patch v.2Splinter Review
Yes, that does make more sense
Attachment #8475152 - Flags: review?(jfkthame)
Attachment #8475152 - Flags: review?(jfkthame) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ae173319e56

No test for now, but when bug 428037 is fixed, we should add one.
Depends on: 428037
Flags: in-testsuite?
https://hg.mozilla.org/mozilla-central/rev/8ae173319e56
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Attachment #8475083 - Flags: review?(jfkthame)
Flags: qe-verify+
Reproduced with Nightly 2014-08-17 on Mac OS X 10.9.5 - the selection is printed as a blank page; same result with Print to file option.
With Firefox 34 beta 8 (Build ID: 20141110195804) the selection is displayed when printing or print to file, but if I select, for e.g., "Mozilla Firefox is free and open source software from the non-profit Mozilla Foundation. Kn" text via about:home URL, the whole text ("Mozilla Firefox is free and open source software from the non-profit Mozilla Foundation. Know your rights…") is printed. Reproducible on all platforms: Mac OS X 10.9.5, Ubuntu 14.04 x64 and Window 7 x64. 
Simon, is this expected at all?

Note:
This is not reproducible on Chrome via the Print Options/Selection only.
Flags: needinfo?(smontagu)
I think that is bug 428111 hitting you. Do you get the same results on a build before the regression date for this bug?
Flags: needinfo?(smontagu)
(In reply to Simon Montagu :smontagu from comment #12)
> I think that is bug 428111 hitting you. Do you get the same results on a
> build before the regression date for this bug?

Yes, indeed, I'm hitting bug 428111 - I got the same results (as in comment 11) with Nightly 2014-07-23.

Although, this issue is still reproducible with latest Nightly 36.0a1 (Build ID: 20141112030202) on Windows 7 x64, e10s enabled/disabled - print selection is broken on about:home (got a blank page). Not reproducible with latest Aurora 35.0a2. May I file a new bug on this matter?
Flags: needinfo?(smontagu)
(In reply to Alexandra Lucinet, QA Mentor [:adalucinet] from comment #13)
> 
> Although, this issue is still reproducible with latest Nightly 36.0a1 (Build
> ID: 20141112030202) on Windows 7 x64, e10s enabled/disabled - print
> selection is broken on about:home (got a blank page). Not reproducible with
> latest Aurora 35.0a2. May I file a new bug on this matter?

Yes, please do.
Flags: needinfo?(smontagu)
(In reply to Simon Montagu :smontagu from comment #14)
> (In reply to Alexandra Lucinet, QA Mentor [:adalucinet] from comment #13)
> > 
> > Although, this issue is still reproducible with latest Nightly 36.0a1 (Build
> > ID: 20141112030202) on Windows 7 x64, e10s enabled/disabled - print
> > selection is broken on about:home (got a blank page). Not reproducible with
> > latest Aurora 35.0a2. May I file a new bug on this matter?
> 
> Yes, please do.

Logged bug 1100366.
Marking this accordingly.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.