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

Status: VERIFIED FIXED

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

Description

[Alice0775 White]

[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

Comment 1

[Alice0775 White]

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

Comment 2

[Daniel Holbert [:dholbert]]

smontagu, do you have cycles to take a look?

Comment 3

[Daniel Holbert [:dholbert]]

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.

Comment 4

[Simon Montagu :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)

Comment 5

[Simon Montagu :smontagu]

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.

Comment 6

[Simon Montagu :smontagu]

Attachment: Patch

Fixes this bug, and a parallel issue in nsPageFrame.cpp, plus an outdated line that I accidentaly didn't remove in nsTableRowFrame.cpp

Comment 7

[Jonathan Kew [: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.... :(

Comment 8

[Simon Montagu :smontagu]

Attachment: Patch v.2

Yes, that does make more sense

Comment 9

[Simon Montagu :smontagu]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8ae173319e56

No test for now, but when bug 428037 is fixed, we should add one.

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/8ae173319e56

Comment 11

[Ada [:adalucinet]]

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.

Comment 12

[Simon Montagu :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?

Comment 13

[Ada [:adalucinet]]

(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?

Comment 14

[Simon Montagu :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.

Comment 15

[Ada [:adalucinet]]

(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.