The default bug view has changed. See this FAQ.

Combo boxes/Dropdown lists/SELECT elements do not print completely

VERIFIED FIXED in mozilla1.8rc1

Status

()

Core
Layout: Form Controls
P1
normal
VERIFIED FIXED
12 years ago
11 years ago

People

(Reporter: Neil P. Pena, Assigned: bz)

Tracking

({fixed1.8, intl, regression})

Trunk
mozilla1.8rc1
fixed1.8, intl, regression
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(4 attachments)

(Reporter)

Description

12 years ago
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.
(Reporter)

Comment 1

12 years ago
Created attachment 198710 [details]
Example: Combo box on screen
(Reporter)

Comment 2

12 years ago
Created attachment 198711 [details]
Example: Combo box in printout
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.
Status: UNCONFIRMED → NEW
Component: General → Layout: Form Controls
Ever confirmed: true
Keywords: intl, regression
OS: Windows XP → All
Product: Firefox → Core
QA Contact: general → layout.form-controls
Hardware: PC → All
Summary: Combo boxes do not print completely → Combo boxes/Dropdown lists/SELECT elements do not print completely
Version: unspecified → Trunk
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.
FYI: This bug is reproduced on Print Preview.
Is this a regression?  Or something we've always had?
See comment 4. We have a report that said 2005062006 build doesn't have this bug.
Ah, ok.  This regressed between 2005-06-20 06:00 and 2005-06-21 06:00.  Bonsai
query: 
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=20050620+060000&maxdate=20050621+060000&cvsroot=%2Fcvsroot
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...
Blocks: 297389
Flags: blocking1.8rc1?
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

12 years ago
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.
Flags: blocking1.8rc1? → blocking1.8rc1-
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?
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... :(
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.
Attachment #200256 - Flags: superreview?(roc)
Attachment #200256 - Flags: review?(roc)
Comment on attachment 200256 [details] [diff] [review]
Proposed fix

Mats, if you get a chance to review too, that would be great.
Attachment #200256 - Flags: review?(mats.palmgren)
Attachment #200256 - Flags: superreview?(roc)
Attachment #200256 - Flags: superreview+
Attachment #200256 - Flags: review?(roc)
Attachment #200256 - Flags: review+
Assignee: nobody → bzbarsky
Fixed on trunk.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Priority: -- → P1
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.8rc1
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.
Attachment #200256 - Flags: approval1.8rc1?
Attachment #200256 - Flags: approval1.7.13+
Attachment #200256 - Flags: approval1.7.13+

Comment 18

12 years ago
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?
Keywords: qawanted
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

12 years ago
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.
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.
Status: RESOLVED → VERIFIED
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.
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. ;)

Updated

12 years ago
Attachment #200256 - Flags: approval1.8rc1? → approval1.8rc1+
Fixed on branch.
Keywords: fixed1.8
Patch was granted approval1.8rc1, so re-requesting blocking1.8rc1 for tracking purposes.
Flags: blocking1.8rc1- → blocking1.8rc1?
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.
Flags: blocking1.8rc1? → blocking1.8rc1-
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.
Flags: blocking1.8rc1- → blocking1.8rc1?
grr, stupid bugzilla :/

Can somebody minus it again? Mid-air collision messed this up.
Flags: blocking1.8rc1?
This is why you don't just blindly click the "overwrite other people's changes" button in a mid-air...
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.
> 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.

Updated

12 years ago
Keywords: qawanted
*** Bug 314333 has been marked as a duplicate of this bug. ***
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.
Attachment #200256 - Flags: review?(mats.palmgren) → review+
The problem with that patch is that it doesn't work because of http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/generic/nsSimplePageSequence.cpp&rev=3.136&mark=287-291#286 and http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/generic/nsSimplePageSequence.cpp&rev=3.136&mark=208-217#207

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.
You need to log in before you can comment on or make changes to this bug.