Use images for buttons in main chrome UI

RESOLVED FIXED

Status

Fennec Graveyard
General
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: mfinkle, Assigned: mfinkle)

Tracking

Trunk
x86
Windows XP

Details

Attachments

(3 attachments)

We tried using a 3D CSS3-based theme to create image-less buttons, but performance regressed. So, we did a quick replace with a flat CSS-based theme and performance got a lot better.

Now we are going back to using images to create nice 3D buttons.
(Assignee)

Updated

9 years ago
Assignee: nobody → mark.finkle
(Assignee)

Updated

9 years ago
tracking-fennec: --- → ?
Created attachment 379965 [details] [diff] [review]
patch for xul and widget changes

This patch contains the structural part of the changes. Changes to the XUL and JS needed to better support a them with both images (pixel dependent) and physical sizes in millimeters (pixel independent).

I added code to WidgetStack to allow for updating the location of unfrozen widgets, which we need since we don't really know the pixel sizes of things until they are rendered. We don't hard code sizes in XUL anymore, whenever possible.
Attachment #379965 - Flags: review?(gavin.sharp)
Created attachment 379966 [details] [diff] [review]
patch for hildon theme changes

This patch has the required changes to the hildon theme. Changes to CSS and new images.

Once we get this part reviewed, I can create a copy of the finished patch for WinCE.
Attachment #379966 - Flags: review?(gavin.sharp)
Attachment #379965 - Flags: review?(gavin.sharp) → review+
Comment on attachment 379966 [details] [diff] [review]
patch for hildon theme changes

I wonder whether a Toolbar.png-like approach with image-regions would be better for perf. I don't recall whether we've done any testing or reached any conclusions there.

some minor issues:
-padding for the identity box seems broken for non-SSL (text doesn't line up, is too close to image)
-tool-panel-open does't have an active image, should it?

>diff --git a/themes/hildon/browser.css b/themes/hildon/browser.css

> #tool-panel-open {
>+  margin-right: -40px; /* big number just to get padding margins to kick in */

> #tool-panel-close {
>+  margin-left: -40px; /* big number just to get padding margins to kick in */

I don't really understand these comments, maybe you should elaborate?

> #tabs {

>+  padding: 0;
>+  padding-top: 0; /* align to top of web content */

Why are both needed?

> richlistitem[type="documenttab"] {
>-  -moz-appearance: none;

Do we really not want that anymore?

>diff --git a/themes/hildon/browser.css.rej b/themes/hildon/browser.css.rej
>new file mode 100644
>--- /dev/null
>+++ b/themes/hildon/browser.css.rej

Er, don't land this :)

>diff --git a/themes/hildon/jar.mn b/themes/hildon/jar.mn

>-  images/tag.png                       (images/tag.png)

Seems like there's still a reference to this one in browser.css:
./browser.css:  list-style-image: url("chrome://browser/skin/images/tag.png");

though I guess we don't use it because of bug 470829 anyways.

>+  images/unlock-40.png                 (images/unlock-40.png)
>+  images/tab-default.png               (images/tab-default.png)
>+  images/tab-active.png                (images/tab-active.png)

These appear to be unused?
Attachment #379966 - Flags: review?(gavin.sharp) → review+
(In reply to comment #3)
> (From update of attachment 379966 [details] [diff] [review])
> I wonder whether a Toolbar.png-like approach with image-regions would be better
> for perf. I don't recall whether we've done any testing or reached any
> conclusions there.

I think the Toolbar.png like approach would be ok, if it was vertically designed. Horizontal image regions can cause slowdowns because the memory is not contiguous. See Stuart for more details.

> some minor issues:
> -padding for the identity box seems broken for non-SSL (text doesn't line up,
> is too close to image)

Fixing in bug 489433 (larry)

> -tool-panel-open does't have an active image, should it?

Nope, we never show it active. We also never show tool-panel-close in the active state. The images are designed to handle the correct "look"

> >diff --git a/themes/hildon/browser.css b/themes/hildon/browser.css
> 
> > #tool-panel-open {
> >+  margin-right: -40px; /* big number just to get padding margins to kick in */
> 
> > #tool-panel-close {
> >+  margin-left: -40px; /* big number just to get padding margins to kick in */
> 
> I don't really understand these comments, maybe you should elaborate?

Adding more, but here's the details: These buttons are designed to "run off" the edge of the window. *-open is in a right-side strip, *-close is in a left-side strip. The strips have -moz-box-align such that the alignment is opposite of the edge the buttons should "run-off". I use a too big negative margin to just make sure the buttons are clipped by the edge of the hbox (maybe I could use overflow-x somehow instead). Mainly an experiment that gave me the intended effect. Could probably be done better.

> > #tabs {
> 
> >+  padding: 0;
> >+  padding-top: 0; /* align to top of web content */
> 
> Why are both needed?

They're not. Removed padding-top

> > richlistitem[type="documenttab"] {
> >-  -moz-appearance: none;
> 
> Do we really not want that anymore?

Oops. Added back

> >diff --git a/themes/hildon/browser.css.rej b/themes/hildon/browser.css.rej
> >new file mode 100644
> >--- /dev/null
> >+++ b/themes/hildon/browser.css.rej
> 
> Er, don't land this :)

Oops. Removed. A little too hg addremove happy

> >diff --git a/themes/hildon/jar.mn b/themes/hildon/jar.mn
> 
> >-  images/tag.png                       (images/tag.png)
> 
> Seems like there's still a reference to this one in browser.css:
> ./browser.css:  list-style-image: url("chrome://browser/skin/images/tag.png");
> 
> though I guess we don't use it because of bug 470829 anyways.

I changed the reference to "tag-default-30.png". Also changed the "starred" imgae to "star-default-30.png". These are both in prep for bug 470829.

> >+  images/unlock-40.png                 (images/unlock-40.png)
> >+  images/tab-default.png               (images/tab-default.png)
> >+  images/tab-active.png                (images/tab-active.png)
> 
> These appear to be unused?

the unlock-40.png and lock-40.png are for the new larry (bug 489433). The "tab-*.png" images have been removed since we switched to non-images for styling them.
http://hg.mozilla.org/mobile-browser/rev/d5db27d4be1a
http://hg.mozilla.org/mobile-browser/rev/ccb35acec348
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Forgot I need to post the WinCE part of the patch too
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Found a silly CSS error. pushed fix:
http://hg.mozilla.org/mobile-browser/rev/87e6f4cf0dc4

Updated

9 years ago
tracking-fennec: ? → 1.0+
Created attachment 382572 [details] [diff] [review]
patch for wince theme changes

This is a patch for the themes\wince changes:
* all images should match those used in hildon
* browser.css and platform.css should match (except for the very top of platform.css where we have some maemo font names)
* jar.mn has the new list of images

no new changes were made. just bringing the wince side up to date.
Attachment #382572 - Flags: review?(pavlov)

Updated

9 years ago
Attachment #382572 - Flags: review?(pavlov) → review+
http://hg.mozilla.org/mobile-browser/rev/64ce55ad512e
Status: REOPENED → RESOLVED
Last Resolved: 9 years ago9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.