566440 - APP Update dialog is too wide with modern

Status: RESOLVED FIXED

(SeaMonkey :: Themes, defect)

Description

[Matthias Versen [:Matti]]

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.3a5pre) Gecko/20100516 SeaMonkey/2.1a2pre

1) select modern as Theme
2) select help/check for Updates
3) notice that the update dialog uses nearly the whole width of the monitor.

Comment 1

[u331436]

confirmed

Comment 2

[Stefan [:stefanh]]

The dialog is mozilla/toolkit/mozapps/update/content/updates.xul and it has width and height set to auto.

Comment 3

[[e-mail]]

Bug 480178 moved the dimensions from locale to skin, presumably the skin part patch needs to be adapted for Modern.

Comment 4

[Edmund Wong (:ewong)]

Taking this bug for a spin.

Comment 5

[Edmund Wong (:ewong)]

Attachment: Changed APP Update dialog to a normal width.

re: Comment #2,  it was not suggested to remove/change that style line in updates.xul, so I opted for this workaround.

Comment 6

[Robert Kaiser]

Comment on attachment 501536 [details] [diff] [review]
Changed APP Update dialog to a normal width.

I'm really not a good reviewer for Modern, let's move this over to Neil.

Comment 7

[[e-mail]]

Comment on attachment 501536 [details] [diff] [review]
Changed APP Update dialog to a normal width.

It's the right start, but the dialog still doesn't look quite right yet. You probably need to incorporate more of the changes from http://hg.mozilla.org/mozilla-central/diff/58de25578e9b/toolkit/themes/winstripe/mozapps/update/updates.css (but note that some of them were subsequently backed out). Unfortunately you can't just feed the files into kdiff3 :-(

Comment 8

[Jens Hatlak (:InvisibleSmiley)]

I think we should really fix at least the size for 2.1, so let's put this on the radar.

(In reply to comment #7)

> You probably need to incorporate more of the changes from (...)

If I compare with the wizards we have (Bookmark Manager Import, Sync Setup) we might get away with doing the following in accordance with the Import wizard: ("title" = heading with white text color and dark blue background color)

1. remove the margin above the title
2. remove the margin to the left and right of the title
3. increase the height of the title
4. align the title text top left (with some padding)
5. add some margin below the title
6. increase the text size for the message below the title.

We might get the above with actually removing CSS rules specific to the update dialog (haven't checked that, just a guess).

> Unfortunately you can't just feed the files into kdiff3 :-(

What were you trying to achieve? You can either compare 2-3 local files (or maybe any KIO with KDE) or copy/paste directly into the left/right hand side (at least on Windows). What bugs me is that you cannot give it a patch file.

Comment 9

[Edmund Wong (:ewong)]

Attachment: APP Update dialog is too wide with modern.

Comment 10

[Philip Chee]

Comment on attachment 524943 [details] [diff] [review]
APP Update dialog is too wide with modern.

I did the initial modern css in Bug 493022 on 2009-08-25 so you only need to worry about winstripe changes since then.

Bug 480178 - Billboard should extend to available space and the update UI should be the same width for all locales
Patch 1 and Patch 3

Bug 595388 - Use same padding for content and header in app update ui
Bug 595390 - Don't use bold button labels in app update ui.

>   link:hover:active {
>    color: #FF0000;
>  }
>  
> -link:focus {
> -  border: 1px dotted #000000;
> -}

Might get rid of link:hover:active as well.

> -#moreDetails {
> -  margin-top: 1px;
> -  margin-bottom: 4px;
> +.wizard-buttons-separator {
> +  margin-top: 0 !important;
>    -moz-margin-start: 3px;
>    -moz-margin-end: 5px;
>  }

I think we can get rid of the margin start/end as well like Firefox.

> +#updates[currentpageid="updatesfoundbasic"] .wizard-button[dlgtype="next"],
> +#updates[currentpageid="updatesfoundbillboard"] .wizard-button[dlgtype="next"],
> +#updates[currentpageid="finished"] .wizard-button[dlgtype="finish"],
> +#updates[currentpageid="finishedBackground"] .wizard-button[dlgtype="finish"] {
> +  font-weight: bold;

removed in Bug 595390.

> +#licenseContent {

> +  -moz-margin-start: 6px;
> +  -moz-margin-end: 6px;

Where do these margins come from? They are not in Winstripe.

> +  -moz-appearance: listbox;

We don't generally use OS widgets in Modern, So we probably style this like a Modern listbox. Check with Neil on what he wants here.

> -#incompatibleWarning {
> -  background-color: #FFFFE0;
> -  color: #000000;
> -  border: 2px solid;
> +#incompatibleListbox {
>    -moz-border-top-colors: #BEC3D3 #5D616E;
>    -moz-border-right-colors: #F8FAFE #5D616E;
>    -moz-border-bottom-colors: #F8FAFE #5D616E;
>    -moz-border-left-colors: #BEC3D3 #5D616E;

Without a border we don't need any border colours as well. Check with Neil if we still want the border.

> -  padding: 3px;
> -  margin: 1px 5px 4px;
> 

> +  -moz-margin-start: 6px;
> +  -moz-margin-end: 6px;
> 
Why these start/end margins? Winstripe got rid of them.

> +  margin-bottom: 6px;
>  }

could do margin: 0px 6px 6px; here.

> #downloadStatusTop, #downloadStatusLine {

downloadStatusTop was removed in Bug 480178 Patch #3.

>  #pauseButton {
> +  list-style-image: url(chrome://mozapps/skin/update/downloadButtons.png);
> +  -moz-image-region: rect(0px, 48px, 16px, 32px);
> ....

> -  list-style-image: url("chrome://mozapps/skin/icons/buttons.png");
> -  -moz-image-region: rect(0px, 48px, 16px, 32px);

We don't have chrome://mozapps/skin/update/downloadButtons.png
But you can use chrome://communcator/skin/downloads/downloadButtons.png and adjust your -moz-image-region to fit.

> -  margin: 0;
> -  border: 1px solid !important;
> +  border: none;
>    -moz-border-top-colors: transparent !important;
>    -moz-border-right-colors: transparent !important;
>    -moz-border-bottom-colors: transparent !important;
>    -moz-border-left-colors: transparent !important;

>    border-radius: 0 !important;

No borders mean no colours nor radii either. Check with Neil if we still want the border.

>    outline: none !important;
> +  margin: 0;
> +  padding: 0;

Don't think we want to override the inherited padding with 0. Check with Neil.

Comment 11

[Edmund Wong (:ewong)]

Attachment: APP Update dialog is too wide with modern. (v3)

Comment 12

[[e-mail]]

Comment on attachment 526571 [details] [diff] [review]
APP Update dialog is too wide with modern. (v3)

>+#licenseContent, #incompatibleListbox {
>+  -moz-margin-start: 6px;
>+  -moz-margin-end: 6px;
I'm not sure that it's worth changing the margin for the incompatibleListbox. For the #licenseContent, I would try using margin: 2px 4px; (see below).

>+#licenseContent {
>+  -moz-appearance: listbox;
As Ratty points out, this is wrong. (In fact, it's wrong on OS/2, but then again I guess they don't have automatic updates...) What you need to do is to copy the styles for listbox (from listbox.css).

> #pauseButton {
I don't think we need to change these styles.

Comment 13

[[e-mail]]

Attachment: Updated for review comments

I don't know why ewong didn't update this himself...

Comment 14

[Justin Wood (:Callek)]

Comment on attachment 530772 [details] [diff] [review]
Updated for review comments

previous patches had r+ from Neil, this is just an update from Neil to address his and Ratty's review nits. rs+=me

Comment 15

[[e-mail]]

Pushed changeset 789b419bafdf to comm-central.

Pushed changeset 03f2040fe2a5 to releases/comm-2.0