Closed Bug 423806 Opened 17 years ago Closed 17 years ago

Keyhole (combined back forward) for small icons mode on windows

Categories

(Firefox :: Theme, defect, P1)

x86
Windows XP
defect

Tracking

()

VERIFIED FIXED
Firefox 3 beta5

People

(Reporter: faaborg, Assigned: mcdavis941.bugs)

References

Details

Attachments

(4 files, 2 obsolete files)

Attached image Toolbar-small.png (obsolete) (deleted) —
Actually, this doesn't look anything like a keyhole, but I guess the name has stuck. We need to update the windows theme for combined back/forward navigation controls with a unified history menu. I'll attach the new toolbar-small.png and toolbar-small-aero.png files. We want to get this in for beta 5, so we are unfortunately going to need a really fast turn around time.
Flags: blocking-firefox3?
Attached image Toolbar-small-aero.png (obsolete) (deleted) —
Note that the etch in these images is visually a little messed up, but the dimensions will stay the same as these files, so you can use them for creating the patch.
(In reply to comment #0) > Created an attachment (id=310416) [details] > Toolbar-small.png > > Actually, this doesn't look anything like a keyhole, but I guess the name has > stuck. We need to update the windows theme for combined back/forward > navigation controls with a unified history menu. I'll attach the new > toolbar-small.png and toolbar-small-aero.png files. > > We want to get this in for beta 5, so we are unfortunately going to need a > really fast turn around time. > I can take this. I'll wrap up some other stuff I have going on and get started in about an hour.
Status: NEW → ASSIGNED
Assignee: nobody → mcdavis941.bugs
Status: ASSIGNED → NEW
Flags: blocking-firefox3? → blocking-firefox3+
Priority: -- → P1
Status: NEW → ASSIGNED
Flags: blocking-firefox3+ → blocking-firefox3?
Priority: P1 → --
Alex, could you tell me what's your intention for the blank space on the left side of the image files? Will there be another version of the back and forward arrows in that spot, paralleling the layout of Toolbar.png, for use when mode is icons+text?
Attached image Toolbar-small-aero.png (deleted) —
>Alex, could you tell me what's your intention for the blank space on the left >side of the image files? Will there be another version of the back and forward >arrows in that spot, paralleling the layout of Toolbar.png, for use when mode >is icons+text? Ok, I forgot this mode, but seriously, who uses small icons + text anyway, that doesn't make any sense :p Updated toolbar with 16x16 nav controls, the XP toolbar will be using aero icons until we can get some luna ones produced.
Attachment #310416 - Attachment is obsolete: true
Attachment #310417 - Attachment is obsolete: true
Priority: -- → P1
Target Milestone: --- → Firefox 3 beta5
Version: unspecified → Trunk
Attached image Toolbar-small.png (deleted) —
Here is the luna one so that you can land these along with your patch.
Flags: blocking-firefox3? → blocking-firefox3+
(In reply to comment #5) > but seriously, who uses small icons + text anyway, that > doesn't make any sense :p Not me. Small toolbar buttons with text labels are still gigantic.
Attached patch browser.css changes v1 (deleted) — Splinter Review
Attachment #310699 - Flags: review?(gavin.sharp)
(In reply to comment #6) > Created an attachment (id=310678) [details] Shouldn't the back/forward glyphs be green for Luna, so that the action of navigating has a color association, e.g. between the toolbar and the unified back/forward popup's hover icon (or wherever else the non-unified glyphs are used)?
(In reply to comment #9) > (In reply to comment #6) > > Created an attachment (id=310678) [details] [details] > > Shouldn't the back/forward glyphs be green for Luna, so that the action of > navigating has a color association, e.g. between the toolbar and the unified > back/forward popup's hover icon (or wherever else the non-unified glyphs are > used)? Good point but I thought he was saying those are just stand-ins until the Luna version is produced.
>Shouldn't the back/forward glyphs be green for Luna Yeah, I just had to copy over the aero ones for now since we don't have any luna source material. This will be corrected in a nightly after beta 5.
Comment on attachment 310699 [details] [diff] [review] browser.css changes v1 >Index: mozilla/browser/themes/winstripe/browser/browser.css >-toolbar[iconsize="large"][mode="icons"] #back-button:not([disabled="true"]):active { >+toolbar[iconsize="large"][mode="icons"] #back-button:not([disabled="true"]):hover:active { What are all these changes for? They look unrelated. http://gavinsharp.com/tmp/skey.png is the intended look, right? Wasn't sure whether they were supposed to be touching.
Attachment #310699 - Flags: review?(gavin.sharp) → review+
Attached image sample showing -moz-image-regions (deleted) —
(In reply to comment #12) > http://gavinsharp.com/tmp/skey.png is the intended look, right? Wasn't sure > whether they were supposed to be touching. Just to make sure I'm going for the right image regions, here's what I'm using. The purple border is included in the displayed image region.
(In reply to comment #13) > > http://gavinsharp.com/tmp/skey.png is the intended look, right? If that's to me, then yes. That is what I meant to do based on what I think Alex wanted. Alex, is this right?
(In reply to comment #12) >> >-toolbar[iconsize="large"][mode="icons"] #back-button:not([disabled="true"]):active { > >+toolbar[iconsize="large"][mode="icons"] #back-button:not([disabled="true"]):hover:active { > > What are all these changes for? They look unrelated. > This is to make the active-state behavior and appearance consistent with the other toolbar buttons in the case when you click and hold on a button but then drag off before releasing. (For anybody who hasn't seen this, click on the refresh button but don't release, and keeping the mouse down, roll off the refresh button and over the content area before releasing. You'll see it only shows active state appearance when the mouse is actually over the button.) I had to add new rules for the small unified case, then took care of the large unified case at the same time so everybody's consistent. I'll file a separate bug for that if you want.
adding in-litmus? - attachment 310737 [details] shows the appropriate -moz-regioning. Should work for inactive, active, hover and depressed states.
Flags: in-litmus?
Attachment #310699 - Flags: approval1.9b5?
Comment on attachment 310699 [details] [diff] [review] browser.css changes v1 a=beltzner
Attachment #310699 - Flags: approval1.9b5? → approval1.9b5+
>If that's to me, then yes. That is what I meant to do based on what I think >Alex wanted. >Alex, is this right? Sorry about the lag, looks great.
Depends on: 424028
Checking in browser/themes/winstripe/browser/browser.css; /cvsroot/mozilla/browser/themes/winstripe/browser/browser.css,v <-- browser.css new revision: 1.189; previous revision: 1.188 done
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
current backward/forward button (attachment 310678 [details]) is too big (24px) than other small (16px) toolbar buttons. The user already switched from default big button because s/he wanted a small one.
Well, I think it should be 22px high, like the location bar. The top border looks odd, so that's the first thing that we should remove.
Verified fix om Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008041004 Minefield/3.0pre
Status: RESOLVED → VERIFIED
errr... i mean Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008041005 Minefield/3.0pre
Blocks: 429062
I think Biju filed bug 429062 about his complaint in comment 21.
Flags: in-litmus? → in-litmus+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: