830342 - Scale combobox is broken in print-preview, after selecting "Custom" and then hitting "Cancel" on its spawned dialog

Status: RESOLVED FIXED

(Core :: Print Preview, defect)

Description

[Ada [:adalucinet]]

Attachment: Scale bar

Mozilla/5.0 (X11; Linux i686; rv:21.0) Gecko/20130114 Firefox/21.0
Build ID: 20130114031033

1. Launch latest Nightly (FF 21a1). 
2. Go to File -> Print Preview and click the Scale bar.
3. Go to Custom and press Cancel.

Expected results:
Scale bar is displayed accordingly.

Actual results:
Scale bar is displayed with rendering issue.

Here is the range i've found:
Last good nightly: 2011-10-21 
First bad nightly: 2011-10-22 
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=2afc252b4d39&tochange=e79245e249c4

Comment 1

[Daniel Holbert [:dholbert]]

Confirmed, w/ Mozilla/5.0 (X11; Linux x86_64; rv:21.0) Gecko/20130114 Firefox/21.0

Mozilla-Inbound regression range is 1 day earlier:
Last good nightly: 2011-10-20
First bad nightly: 2011-10-21
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=eb8d12d360db&tochange=fb302f2dd9c2

Comment 2

[Daniel Holbert [:dholbert]]

From hg bisect -- the first bad revision is:
{
changeset:   79015:da852a7882b5
user:        Peter Van der Beken <[e-mail]>
date:        Thu Oct 13 15:36:09 2011 +0200
summary:     Fix for bug 693258 (Fixes for test failures when turning off new list DOM bindings). r=bz/Waldo.
https://hg.mozilla.org/mozilla-central/rev/da852a7882b5
}

That cset exhibits the issue; its parent (f7ed9d64a8d7) doesn't.

Comment 3

[Peter Van der Beken [:peterv]]

This is probably due to the change in GetArrayIndexFromId, still debugging.

Comment 4

[Daniel Holbert [:dholbert]]

Thanks for investigating!

One more thing I meant to mention: prior to the regression, the STR triggered different arguably-broken behavior -- after you hit "Cancel", the combobox would jump to some arbitrary value (usually "30%" I think), for no apparent reason. (Still, that's better than making the combobox collapse in on itself.)

Comment 5

[Peter Van der Beken [:peterv]]

(In reply to Daniel Holbert [:dholbert] from comment #4)
> One more thing I meant to mention: prior to the regression, the STR
> triggered different arguably-broken behavior -- after you hit "Cancel", the
> combobox would jump to some arbitrary value (usually "30%" I think), for no
> apparent reason. (Still, that's better than making the combobox collapse in
> on itself.)

The issue is that the scale method is passing an empty string to the menulist's selectedIndex setter. That setter then sets selectedItem to childNodes[val] with val being the empty string. In the past we incorrectly treated an empty string as 0, but we're now following WebIDL and treating it as an invalid index. I have a fix for this in the scale method. I've considered fixing the selectedItem setter to do the equivalent of EcmaScript's ToUint32 on val before using it as an index, but that brings back the old behaviour that you noted.

Comment 6

[Peter Van der Beken [:peterv]]

Attachment: v1

Comment 7

[Peter Van der Beken [:peterv]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f98f9cf410fe

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f98f9cf410fe