501095 - Clean up appPicker's use of XUL

Status: RESOLVED FIXED

(Toolkit :: Preferences, defect)

Description

[[e-mail]]

Bug 498153 prompted me to look at the XUL underlying the app picker.

Comment 1

[[e-mail]]

Attachment: Proposed patch

I wondered why it went to all the trouble of creating custom XUL that is already readily available via the listbox element.

I switched from overloading the value property to using a handlerApp property (this hack only works because the XBL gets bound later).

I stopped setting identical ids on multiple elements. In fact I don't set any ids at all! I did change the listbox's ID to avoid breaking bug 498153.

I removed an unnecessary vbox, and other minor tweaks.

I also moved the placement of the Browse button, but I can readily revert that.

Comment 2

[Jim Mathies [:jimm]]

(In reply to comment #1)
> Created an attachment (id=385715) [details]
> Proposed patch
> 
> I wondered why it went to all the trouble of creating custom XUL that is
> already readily available via the listbox element.
> 
> I switched from overloading the value property to using a handlerApp property
> (this hack only works because the XBL gets bound later).
> 
> I stopped setting identical ids on multiple elements. In fact I don't set any
> ids at all! I did change the listbox's ID to avoid breaking bug 498153.
> 
> I removed an unnecessary vbox, and other minor tweaks.
> 
> I also moved the placement of the Browse button, but I can readily revert that.

Probably because this was the very first xul dialog I ever authored. :) Although it did make it through reviews. :O

Comment 3

[Asaf Romano (gone)]

Comment on attachment 385715 [details] [diff] [review]
Proposed patch

r=mano.

Comment 4

[[e-mail]]

Pushed changeset acd5dee9dab3 to mozilla-central.

Comment 5

[[e-mail]]

Comment on attachment 385715 [details] [diff] [review]
Proposed patch

>+.listcell-label {
>+  margin: 5px;
Unfortunately a listcell-label has a margin: 0px !important (and some padding) so this rule has no effect. Which would you prefer,
a) Just remove the margin rule
b) Replace the margin with 5px padding
c) Something else?

Comment 6

[[e-mail]]

Attachment: Spacing fix

Patch following b) above, since that was Gavin's "obvious choice".

I also swapped the width and height into toolkit's normal order.

Comment 7

[[e-mail]]

Comment on attachment 421065 [details] [diff] [review]
Spacing fix

Pushed changeset cea14ad20b61 to mozilla-central.

Comment 8

[Bryan]

Could this have caused the file (view, bookmarks and help exhibit this new behavior) menus to shift left and right when navigating between file menu with sub-folders?  I'm seeing a one/two pixel shift left and right using: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20100111 Minefield/3.7a1pre ID:20100111171142

~B

Comment 9

[[e-mail]]

These patches only ever affected one dialog used in certain places on Windows.