Multiple printing issues on google maps on Mac OSX

RESOLVED FIXED

Status

()

Core
Printing: Output
P2
major
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: tchung, Assigned: roc)

Tracking

({relnote})

Trunk
PowerPC
Mac OS X
relnote
Points:
---
Bug Flags:
blocking1.9 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(5 attachments)

(Reporter)

Description

11 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X; es-ES; rv:1.9a9pre) Gecko/2007092404 Minefield/3.0a9pre
Build Identifier: 

There are numerous printing issues when searching for a To/From location on maps.google.com:

1) All text on the printed sheets are blacked out.  Unreadable. (start/end header, turn by turn directions, notes textbox)
2) The Turn by turn directions are completely missing.  Only the smaller map images are present
3) A third page is printed for no reason.  It's completely blank. (pg 3 of 3)

Note: The above is printed in Portrait setting.  Tested on Mac and Windows.

Note 2: this prints and works as expected on Firefox 2.0.0.7.   The issues listed above are not there.

Reproducible: Always

Steps to Reproduce:
1. install latest minefield trunk nightly
2. go to maps.google.com, and search for a Start/End addresses
3. Print to printer
4. Verify the issues above.  Lots of printer problems.
Actual Results:  
Printer issues on maps.google.com.  No repro on 2.0.0.7 builds.

Expected Results:  
No printer issues.
(Reporter)

Comment 1

11 years ago
adding blocking flag.  this is a critical User experience, as maps.google.com is a popular site.
Flags: blocking1.9?

Updated

11 years ago
Version: unspecified → Trunk
Can you retry this with a newer nightly?

Though to separate bugs should be filed -- one for Windows and one for Mac, as the printing backends are very different.

Updated

11 years ago
Flags: blocking1.9? → blocking1.9+
(Reporter)

Comment 3

11 years ago
STR 1,2, and 3 are still repro'ing on Mozilla/5.0 (Macintosh; U; Intel Mac OS X; es-ES; rv:1.9a9pre) Gecko/2007100804 Minefield/3.0a9pre

I'll test this on windows, and file a separate bug if still repros.
(Reporter)

Comment 4

11 years ago
changing this bug to mac only.  will open a separate bug on windows per comment #2
OS: All → Mac OS X
Hardware: All → Macintosh
(Reporter)

Updated

11 years ago
Blocks: 399238
(Reporter)

Updated

11 years ago
Summary: Multiple printing issues on google maps → Multiple printing issues on google maps on Mac OSX

Updated

11 years ago
Priority: -- → P2

Comment 5

11 years ago
the missing text on page 2 is probably the same as the windows bug.  not sure about the other bits
I tried this in my trunk build (clicking on the "Print" link to print, not a direct Cmd-P) and I get a lovely PDF:
http://people.mozilla.com/~roc/bug379428.pdf
Looks perfect. You can even select the text. Too big to attach here though.
I tried doing Cmd-P just in case and I get:
http://people.mozilla.com/~roc/bug379428-cmdP.pdf
Which is also perfect, as far as I can tell.
(Reporter)

Comment 8

11 years ago
Hey robert,

i noticed you dont have the Start and End image on your pdf, which exists on page 2.  Can you try enabling that option and try again?    Perhaps thats the culprit.
(Reporter)

Comment 9

11 years ago
For me, i see the exact same results on Mac OSX as i do for windows.  Refer to screenshots at: https://bugzilla.mozilla.org/show_bug.cgi?id=399238#c4
Okay, I can reproduce this now. The precise steps are:
-- Get the directions
-- Click on the "Print" link
-- Enable "show original map view" in the checkbox at the top of the page (this is key)
-- Print (or print preview)
Created attachment 287053 [details]
kind-of reduced testcase

This testcase shows the bug for me. I removed all <script>s and all references to external files. It could still use considerable reduction --- currently 1318 lines...
Created attachment 287054 [details]
more reduction

Down to 1043 lines... I'll have to carry on with this later, unless someone wants to take over the fun job...
Created attachment 287074 [details]
really reduced testcase!

This is really reduced! The print preview is three blank pages, but it should be one blank page followed by a page with "Head" on it.
Created attachment 287081 [details] [diff] [review]
fix

This fixes the testcase and Google Maps.

Here's what seems to be happening:

1) The page-break-after property causes creation of an nsPageBreakFrame which sets its height to the available height, eating up all the height of the page. This approach to page-break-after is entirely bogus, but oh well.

2) We carry on trying to place content at the bottom of the first page! This is the way it's supposed to work, believe it or not. The idea is that as soon as we find content that needs some height, it will be pushed to the next page.

3) In this case, we try to place the outer table, so we reflow it with zero available height. That leads us to reflow the inner table with zero available height. There isn't enough space so the inner table body calls nsTableRowGroupFrame::SplitRowGroup.

4) SplitRowGroup reflows the row again. The block frame containing the two <br>s breaks vertically after the first <br>, but the first <br> doesn't fit anyway, so it returns a reflow status of NS_FRAME_NOT_COMPLETE and NS_FRAME_TRUNCATED. This status propagates back up to SplitRowGroup. SplitRowGroup sets rowIsOnPage to PR_FALSE, to indicate that none of the row fits on this page, which is correct, and it therefore does not create a continuation for the row, since we'll try to make the row fit completely on the next page. This is also correct. But it allows the row's reflow status to return to nsTableRowGroup::Reflow and then out as the rowgroup's reflow status. This where we first go seriously wrong.

5) ... because the table, seeing that the rowgroup returns NS_FRAME_NOT_COMPLETE, creates a continuation for the rowgroup.

6) The whole outer table is eventually pushed to the second page because of course none of it fit at the bottom of the first page. We reflow the inner table again. The first row now fits, but nsTableRowGroupFrame::Reflow has this to say:
  // If we have a next-in-flow, then we're not complete
  // XXXldb This used to be done only for the incremental reflow codepath.
  if (GetNextInFlow()) {
    aStatus = NS_FRAME_NOT_COMPLETE;
  }
Indeed the rowgroup has a next-in-flow, albeit one that's empty and has no rows --- the one we created in step 5. So the rowgroup continues to return NS_FRAME_NOT_COMPLETE even though it really is complete.

7) This effectively causes another vertical break at the bottom of the inner table. I'm not sure why "Head" doesn't appear on the third page, but it's probably related to the bogosity of this whole situation and I'm not motivated to debug further given that things have already gone hopelessly wrong.

There are several things wrong here, including the nsTableRowGroupFrame::Reflow code I quoted, but the easiest fix (and I think the safest) is at step 4. If you look at nsTableRowGroupFrame::SplitRowGroup, there are other paths that handle the "we can't push children" case, and they all set aStatus to NS_FRAME_COMPLETE, which is logical because we want to push this entire rowgroup to a new page and try to reflow the row again there instead of splitting our rowgroup. (Those paths also call UndoContinuedRow to destroy a row continuation that was already created; we don't need to do that in my patch because when rowIsOnPage is false, we didn't create a row continuation. The assertion checks that.)

The big picture is that table splitting is poorly/incompletely implemented, even for just the non-incremental/non-dynamic case, even though what we have is already horrendously complex. The continuation model itself is a flawed design.

Not sure who should review this. Let's try Bernd and Boris.
Assignee: nobody → roc
Status: NEW → ASSIGNED
Attachment #287081 - Flags: superreview?(bzbarsky)
Attachment #287081 - Flags: review?(bernd_mozilla)

Comment 15

11 years ago
Comment on attachment 287081 [details] [diff] [review]
fix

I think this is correct, I first saw the patch and thought thats it, and only then the lengthy comment, it looks to me like a clear violation of the contract.

>The continuation model itself is a flawed design.

Robert you are old enough to know the sad history (bug 165772) how this has happened.

There are more bad things here (bug 373400) which slips due to my inability to devote more time to mozilla.
Attachment #287081 - Flags: review?(bernd_mozilla) → review+
Thanks Bernd!

This bad decision was made in 1998 or before, really...
Whiteboard: [needs review]
Comment on attachment 287081 [details] [diff] [review]
fix

sr=bzbarky
Attachment #287081 - Flags: superreview?(bzbarsky) → superreview+
Whiteboard: [needs review] → [needs approval/landing]
Created attachment 288117 [details] [diff] [review]
reftests

This can land with the patch

Comment 19

11 years ago
Comment on attachment 287081 [details] [diff] [review]
fix

a+ - note that blocking bugs do not need patch approvals...
Attachment #287081 - Flags: approval1.9? → approval1.9+
Checked in
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Flags: in-testsuite+
Whiteboard: [needs approval/landing]
(Reporter)

Updated

11 years ago
Keywords: relnote
(Reporter)

Comment 21

11 years ago
Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b2pre) Gecko/2007111404 Minefield/3.0b2pre.  

relnote for Beta 1
You need to log in before you can comment on or make changes to this bug.