Last Comment Bug 311380 - Combo boxes/Dropdown lists/SELECT elements do not print completely
: Combo boxes/Dropdown lists/SELECT elements do not print completely
Status: VERIFIED FIXED
: fixed1.8, intl, regression
Product: Core
Classification: Components
Component: Layout: Form Controls (show other bugs)
: Trunk
: All All
: P1 normal (vote)
: mozilla1.8rc1
Assigned To: Boris Zbarsky [:bz] (still a bit busy)
:
:
Mentors:
http://forums.beyondunreal.com/forumd...
: 314333 (view as bug list)
Depends on:
Blocks: 297389
  Show dependency treegraph
 
Reported: 2005-10-06 11:30 PDT by Neil P. Pena
Modified: 2006-08-15 16:25 PDT (History)
13 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Example: Combo box on screen (118.21 KB, image/jpeg)
2005-10-06 11:30 PDT, Neil P. Pena
no flags Details
Example: Combo box in printout (48.60 KB, image/jpeg)
2005-10-06 11:31 PDT, Neil P. Pena
no flags Details
Patch that would work if printing sucked less (2.01 KB, patch)
2005-10-19 22:13 PDT, Boris Zbarsky [:bz] (still a bit busy)
no flags Details | Diff | Splinter Review
Proposed fix (1.24 KB, patch)
2005-10-20 12:11 PDT, Boris Zbarsky [:bz] (still a bit busy)
roc: review+
mats: review+
roc: superreview+
asa: approval1.8rc1+
Details | Diff | Splinter Review

Description Neil P. Pena 2005-10-06 11:30:17 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4

When printing out a web page that has a combo box on it, the text in the combo
box is cut off after the first whitespace.

Reproducible: Always

Steps to Reproduce:
1. Go to a page that has a combo box with an item having a space in it.  (An
example is at http://forums.beyondunreal.com/forumdisplay.php?f=12, at the
bottom of the page)
2. Select that item, if it is not already selected.
3. Print the page.

Actual Results:  
The text in the combo box only shows the first word of the item.

Expected Results:  
The entire combo box text should be printed.
Comment 1 Neil P. Pena 2005-10-06 11:30:52 PDT
Created attachment 198710 [details]
Example: Combo box on screen
Comment 2 Neil P. Pena 2005-10-06 11:31:09 PDT
Created attachment 198711 [details]
Example: Combo box in printout
Comment 3 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-10-14 21:46:44 PDT
We can reproduce with intl page. If select elements have non-ASCII character in
its label, the label's first character is only printed.
http://bugzilla.mozilla.gr.jp/query.cgi

This can be reproduced on Mac too.
Comment 4 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-10-14 21:51:20 PDT
1.8 branch has this bug. I think that this is serious problem for printing.
An Japanese tester said:
Firefox/2005062006-trunk/WinXP O.K.
Firefox/2005070106-trunk/WinXP buggy.
Comment 5 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-10-14 21:53:46 PDT
FYI: This bug is reproduced on Print Preview.
Comment 6 Boris Zbarsky [:bz] (still a bit busy) 2005-10-14 22:27:45 PDT
Is this a regression?  Or something we've always had?
Comment 7 Masayuki Nakano [:masayuki] (Mozilla Japan) 2005-10-14 22:31:09 PDT
See comment 4. We have a report that said 2005062006 build doesn't have this bug.
Comment 9 Boris Zbarsky [:bz] (still a bit busy) 2005-10-14 23:03:10 PDT
My guess is this is fallout from bug 297389 somehow.  That 'Y' is the first
Unichar in the string cast to char, but I have no idea how _that_ happens...
Comment 10 Boris Zbarsky [:bz] (still a bit busy) 2005-10-15 06:25:12 PDT
One possibility might be that we post the event, but then disconnect from the
document ('cause printing is like that), so the text change when the event fires
is not propagated to the frame and as a result the frame has bogus offsets as
compared to the actual text in the content node....
Comment 11 Asa Dotzler [:asa] 2005-10-17 11:23:33 PDT
Mats, if you can come up with a low risk patch that fixes this in the next few
days, we'd consider taking it for the release but this isn't going to block.
Comment 12 Boris Zbarsky [:bz] (still a bit busy) 2005-10-19 20:56:53 PDT
We might be able to work around this with that "spin an event loop till all
pending PLEvents are done for printing" hack I recall we had at some point...
shaver, do you happen to know what file that was in?
Comment 13 Boris Zbarsky [:bz] (still a bit busy) 2005-10-19 22:13:10 PDT
Created attachment 200171 [details] [diff] [review]
Patch that would work if printing sucked less

Unfortunately, the page sequence frame ignores all but the initial reflow. 
After that it never reflows its kids.  So the reflow that would update our
textframes is stopped before it ever gets to them.

I can't make enough sense of the mess that is nsSimplePageSequenceFrame at this
time of night.	I might be able to look at this before Sunday, but I really
doubt it.  Help with this would be much appreciated... :(
Comment 14 Boris Zbarsky [:bz] (still a bit busy) 2005-10-20 12:11:07 PDT
Created attachment 200256 [details] [diff] [review]
Proposed fix

Just get the right text up front when we're creating the content.  That way we
don't have to worry about having to change the text from reflow and then having
to do a second reflow...

This should be quite safe, but if people are worried I'm willing to set the
index back to -1 and Truncate() mDisplayedOptionText after calling
ActuallyDisplayText.  I really don't think that's needed, though.

I did verify that we're guaranteed a mListControlFrame by the time we get here.
Comment 15 Boris Zbarsky [:bz] (still a bit busy) 2005-10-20 12:11:41 PDT
Comment on attachment 200256 [details] [diff] [review]
Proposed fix

Mats, if you get a chance to review too, that would be great.
Comment 16 Boris Zbarsky [:bz] (still a bit busy) 2005-10-20 16:19:49 PDT
Fixed on trunk.
Comment 17 Boris Zbarsky [:bz] (still a bit busy) 2005-10-20 16:20:59 PDT
Comment on attachment 200256 [details] [diff] [review]
Proposed fix

Requesting branch approval.  This just sets the text to the right thing up
front before layout has happened, so we don't need a second pass of reflow. 
This should be quite safe, and fixes a problem where our textframes are
basically out of sync with our content nodes and reading data from the latter
in a way inconsistent with the way that data is actually stored.
Comment 18 Asa Dotzler [:asa] 2005-10-20 17:54:07 PDT
bz, we can consider this for the branch after it's been verified on the trunk.
What kind of testing do we need to do on the trunk to feel confident about
taking this (what could break?) Is it all in printing or could there be page
layout regressions?
Comment 19 Boris Zbarsky [:bz] (still a bit busy) 2005-10-20 18:57:56 PDT
The thing to test would be comboboxes.  Those should be the only thing affected.
 The thing to really test would be comboboxes that get inserted into the DOM
(which constructs the frame and runs the code I'm changing) and then have the
selected item modified by script before they actually lay out.  Once we lay them
out the first time, we set the selected index and text (in Reflow()), and then
this patch no longer matters.

I did do some testing of that sort of thing, and it all worked.  But if others
can test too, that would be great.

That's as far as page layout regressions go.  For printing, the thing to test is
printing of comboboxes, but here of course there is no DOM mutation involved. 
I'm not even sure what to suggest to test; that is, I can't think of things that
might break.
Comment 20 Bob Clary [:bc:] 2005-10-21 11:31:09 PDT
selects print ok for me with a Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US;
rv:1.9a1) Gecko/20051021 Firefox/1.6a1 cvs build.
Comment 21 Marcia Knous [:marcia - use ni] 2005-10-21 12:09:09 PDT
Verified using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1)
Gecko/20051021 Firefox/1.6a1. I printed out the URL cited and the page prints
fine, nothing gets cut off.
Comment 22 Martijn Wargers [:mwargers] (not working for Mozilla) 2005-10-21 12:22:44 PDT
Not really sure what to test here. It seems to work fine on various websites.
I've made sort of a testcase for this:
http://wargers.org/mozilla/bug311380/selectindex.htm
Also works just fine.
Comment 23 Reed Loden [:reed] (use needinfo?) 2005-10-21 17:43:26 PDT
Sorry for the bug spam, but I thought the drivers should see this from a
productivity perspective.

Today, I had printed something off to be turned in but all the combo boxes were
messed up. It's kinda annoying when I'm trying to print a completed application
off to be turned in and all the boxes only have the first word in them. :)

It would definitely be nice to get this fixed for 1.8. ;)
Comment 24 Boris Zbarsky [:bz] (still a bit busy) 2005-10-23 06:30:05 PDT
Fixed on branch.
Comment 25 Reed Loden [:reed] (use needinfo?) 2005-10-23 12:17:39 PDT
Patch was granted approval1.8rc1, so re-requesting blocking1.8rc1 for tracking purposes.
Comment 26 Boris Zbarsky [:bz] (still a bit busy) 2005-10-23 12:22:46 PDT
There's nothing to track.  This is not blocking 1.8rc1.  But it's checked in on the 1.8 branch, so it'll make 1.8rc1.
Comment 27 Reed Loden [:reed] (use needinfo?) 2005-10-23 12:24:56 PDT
Ah, I thought you all wanted to have all the 1.8rc1 bugs to have blocking1.8rc1 too (for some type of tracking). My fault. :)

Sorry for wasting your time.
Comment 28 Reed Loden [:reed] (use needinfo?) 2005-10-23 12:25:53 PDT
grr, stupid bugzilla :/

Can somebody minus it again? Mid-air collision messed this up.
Comment 29 Boris Zbarsky [:bz] (still a bit busy) 2005-10-23 12:26:53 PDT
This is why you don't just blindly click the "overwrite other people's changes" button in a mid-air...
Comment 30 Reed Loden [:reed] (use needinfo?) 2005-10-23 12:33:21 PDT
Well, I called it a mid-air collision, but I guess it wasn't in the definition of the phrase, though it should have been. After I got the e-mail denying blocking, I refreshed the page and added my comment. Somehow I guess it hadn't minused it yet as it re-requested blocking1.8rc1? even though I didn't set it that time. Oh well.
Comment 31 Boris Zbarsky [:bz] (still a bit busy) 2005-10-23 12:52:26 PDT
> I refreshed the page and added my comment.

Use shift-reload when doing that please.  reload preserves form state, even if it has changed on the server.
Comment 32 Martijn Wargers [:mwargers] (not working for Mozilla) 2005-10-29 12:12:49 PDT
*** Bug 314333 has been marked as a duplicate of this bug. ***
Comment 33 Mats Palmgren (:mats) 2006-08-15 01:42:57 PDT
Comment on attachment 200256 [details] [diff] [review]
Proposed fix

Boris, the patch looks good, but I also think we should fix
ReflowPrintObject() like you outlined in your first patch.
I think posting events is a good way to avoid recursive frame
construction and the way ReflowPrintObject currently works blocks
that pattern from being used more.
Comment 35 Boris Zbarsky [:bz] (still a bit busy) 2006-08-15 16:25:18 PDT
Though some of that code is going away on the reflow branch... so maybe after that lands we should revisit this.... But note that we nuked FlushEventQueue() in bug 325991 because it was causing problems.

Note You need to log in before you can comment on or make changes to this bug.