Closed Bug 414732 Opened 16 years ago Closed 15 years ago

minor visual issues with addressbar and site identity button and search button

Categories

(Firefox :: Theme, defect)

All
macOS
defect
Not set
trivial

Tracking

()

VERIFIED FIXED
Firefox 3.6a1

People

(Reporter: asa, Assigned: mstange)

References

Details

(Keywords: polish, verified1.9.1, Whiteboard: [polish-easy] [polish-visual][polish-p1])

Attachments

(5 files, 1 obsolete file)

See attachment for guidance.

1. the shadow for the site identity button where it meets the text field for the addressing area isn't right. It ends too abruptly (at least on the top -- see red circle) and it overlaps with the inner shadow of the text area darketing the corner there a bit too much. 

2. the button's top and bottom shadow do not match the gray of the text field's top and bottom shadow. (see the blue circled area for an example.)
Severity: normal → trivial
Flags: blocking-firefox3?
The bottom circled issue in the screenshot is fixed but there's still a wrong shadow in the top of the address and search fields as seen in the circled area in the screenshot
Flags: wanted-firefox3+
Flags: blocking-firefox3?
Flags: blocking-firefox3-
Kevin, in bug 431853 you said that it will be fixed. Is it true? I cannot really see this issue anymore.
I don't see it either.
Status: NEW → RESOLVED
Closed: 16 years ago
Hardware: PC → All
Resolution: --- → WORKSFORME
At least it's really minor but the bottom line shows up a shadow which doesn't make sense because of the two extra pixels it's too long and end abrupt. I'll attach a screenshot. Alex, something to fix or can we live with it?
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Whiteboard: [polish-easy] [polish-visual]
Attached image with a blank tab
Yeah, I've been seeing this too with current nightlies, in a blank location bar.
I'm also seeing the outer glow get darker on the site button, we should try to fix that as well.  Also effects search button.
Summary: minor visual issues with addressbar and site identity button → minor visual issues with addressbar and site identity button and search button
I'll take this.
Assignee: nobody → mstange
Status: REOPENED → ASSIGNED
Markus, any ETA on a patch here?
Next Wednesday.
Attached patch v1 (obsolete) — Splinter Review
The CSS changes in this patch extend the urlbar / searchbar by 1px to all sides so that the focus ring is smoother.

The patch also fixes bug 480903 because I'm already messing with the padding for #identity-icon-label.
Attachment #365408 - Flags: review?(dao)
Blocks: 480903
Attached image screenshot of patch
top row: before patch, bottom row: after patch
Blocks: 481382
Comment on attachment 365408 [details] [diff] [review]
v1

> #urlbar-icons {
>   -moz-box-align: center;
>-  min-height: 26px;
>+  min-height: 28px;
> }

Does the min-height actually achieve anything? I think you can just drop it.

>--- a/browser/themes/pinstripe/browser/searchbar.css
>+++ b/browser/themes/pinstripe/browser/searchbar.css

> .searchbar-textbox > .autocomplete-textbox-container > .textbox-input-box {
>-  -moz-margin-start: 44px !important;
>+  -moz-margin-start: 45px !important;
> }

That selector is twice in that file, please merge and nuke the !important.
Attached patch v2Splinter Review
(In reply to comment #14)
> (From update of attachment 365408 [details] [diff] [review])
> > #urlbar-icons {
> >   -moz-box-align: center;
> >-  min-height: 26px;
> >+  min-height: 28px;
> > }
> 
> Does the min-height actually achieve anything? I think you can just drop it.

How on earth did you catch that?
You're right, it doesn't have any effect. I dropped it.

> >--- a/browser/themes/pinstripe/browser/searchbar.css
> >+++ b/browser/themes/pinstripe/browser/searchbar.css
> 
> > .searchbar-textbox > .autocomplete-textbox-container > .textbox-input-box {
> >-  -moz-margin-start: 44px !important;
> >+  -moz-margin-start: 45px !important;
> > }
> 
> That selector is twice in that file, please merge and nuke the !important.

Done.
Attachment #365408 - Attachment is obsolete: true
Attachment #365504 - Flags: review?(dao)
Attachment #365408 - Flags: review?(dao)
Comment on attachment 365504 [details] [diff] [review]
v2

Looks good to me, but I can't tell if it's correct in terms of OS X integration.
Attachment #365504 - Flags: ui-review?(faaborg)
Attachment #365504 - Flags: review?(dao)
Attachment #365504 - Flags: review+
Attachment #365504 - Flags: ui-review?(faaborg) → ui-review+
pushed: http://hg.mozilla.org/mozilla-central/rev/59ece877e2fd
Status: ASSIGNED → RESOLVED
Closed: 16 years ago15 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.2a1
Verified fixed with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090310 Minefield/3.2a1pre ID:20090310030639

Markus, is the patch ready to ask for approval1.9.1?
Status: RESOLVED → VERIFIED
Flags: wanted-firefox3.1?
Attachment #365504 - Flags: approval1.9.1?
Comment on attachment 365504 [details] [diff] [review]
v2

a191=beltzner
Attachment #365504 - Flags: approval1.9.1? → approval1.9.1+
Verified fixed with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b4pre) Gecko/20090402 Shiretoko/3.5b4pre ID:20090402031241
Flags: wanted-firefox3.5?
This bug's priority relative to the set of other polish bugs is:
P1 - Polish issue that appears in the main window, or is something that the user may encounter several times a day.

bug was in the main window
Whiteboard: [polish-easy] [polish-visual] → [polish-easy] [polish-visual][polish-p1]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: