389689 - Have the dialog for nsIContentDispatchChooser use the width property

Status: VERIFIED FIXED

(Core Graveyard :: File Handling, defect)

Description

[Shawn Wilsher :sdwilsh]

We had to use style for the initial landing because of some odd bug.  I'll be filing that in just a moment.

End goal is to use width="value" where value is locale dependent.

Comment 1

[Shawn Wilsher :sdwilsh]

ok, we can work around this by specifying a height as well - what fun!

Comment 2

[Shawn Wilsher :sdwilsh]

Attachment: Screenshot without check

Now of course, setting the height means no window.sizeToContent when we hide and show the text under the checkbox...

Comment 3

[Shawn Wilsher :sdwilsh]

Attachment: Screenshot with check

Comment 4

[Shawn Wilsher :sdwilsh]

Attachment: v1.0

and the code...

Comment 5

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 274042 [details]
Screenshot without check

ui-r+

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 274043 [details]
Screenshot with check

ui-r+

Comment 7

[Shawn Wilsher :sdwilsh]

Attachment: v1.1

[5:02pm] Mano: sdwilsh: er, did you mean visibility: hidden; in your patch?
[5:02pm] Waldo is now known as Waldo|lunch.
[5:02pm] sdwilsh: Mano: no - because then the richlist box changes height and it doesn't behave well
[5:03pm] sdwilsh: er
[5:03pm] sdwilsh: wait
[5:03pm] sdwilsh: what does that do again?
[5:03pm] sdwilsh: because maybe now that I think about it
[5:03pm] Mano: should keep the element in its height
[5:03pm] sdwilsh: yeah, that's what I should probably use
[5:04pm] Mano: k, please test
[5:04pm] Mano: sdwilsh: also :not([visible]) is cheaper
[5:04pm] Mano: +removeAttribute, that is.
[5:04pm] sdwilsh: hrm
[5:04pm] Mano: i think this belongs to the content style sheet
[5:05pm] sdwilsh: ok - I suppose that makes more sense
[5:06pm] Mano: paste this log there please.
[5:07pm] sdwilsh: in bug?
[5:07pm] NeilAway left the chat room. (Connection reset by peer)
[5:07pm] sdwilsh: I'll do it with a new patch
[5:08pm] Mano: yeah, so old-gavin would know why young-mano r-ed
[5:08pm] gavin_: haha

Comment 8

[Asaf Romano (gone)]

Comment on attachment 274052 [details] [diff] [review]
v1.1

>Index: toolkit/mozapps/handling/content/dialog.js
>===================================================================
>RCS file: /cvsroot/mozilla/toolkit/mozapps/handling/content/dialog.js,v
>retrieving revision 1.1
>diff -u -8 -p -r1.1 dialog.js
>--- toolkit/mozapps/handling/content/dialog.js	26 Jul 2007 04:24:26 -0000	1.1
>+++ toolkit/mozapps/handling/content/dialog.js	26 Jul 2007 21:12:20 -0000
>@@ -205,20 +205,20 @@ var dialog = {
>     this._okButton.disabled = this._itemChoose.selected;
>   },
> 
>  /**
>   * Updates the UI based on the checkbox being checked or not.
>   */
>   onCheck: function onCheck()
>   {
>-    document.getElementById("remember-text").hidden =
>-      !document.getElementById("remember").checked;
>-
>-    window.sizeToContent();
>+    if (document.getElementById("remember").checked)
>+      document.getElementById("remember-text").setAttribute("visible", "");

nit "true", for consistency.

r=mano otherwise.

Comment 9

[Shawn Wilsher :sdwilsh]

Attachment: v1.2

For checkin

Comment 12

[Shawn Wilsher :sdwilsh]

Comment on attachment 274213 [details] [diff] [review]
v1.2

I don't need approval for this after all.

Comment 13

[Shawn Wilsher :sdwilsh]

Checking in toolkit/mozapps/handling/content/dialog.js;
new revision: 1.3; previous revision: 1.2
Checking in toolkit/mozapps/handling/content/dialog.xul;
new revision: 1.2; previous revision: 1.1
Checking in toolkit/mozapps/handling/content/handler.css;
new revision: 1.2; previous revision: 1.1
Checking in toolkit/mozapps/handling/content/handling.dtd;
new revision: 1.2; previous revision: 1.1

Comment 14

[cmtalbert]

Dialog is covered in Litmus:
https://litmus.mozilla.org/show_test.cgi?searchType=by_category&product_id=1&branch_id=15&testgroup_id=55&subgroup_id=884
https://litmus.mozilla.org/show_test.cgi?searchType=by_category&product_id=1&branch_id=15&testgroup_id=56&subgroup_id=820

Verifying on: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9pre) Gecko/2008050704 Minefield/3.0pre