Closed Bug 989701 Opened 6 years ago Closed 6 years ago
Location bar text sits one pixel too high unless the forward button is enabled
I suspect bug 477948 caused this.
This probably also fixes bug 989466. I still need to verify this.
Comment on attachment 8399482 [details] [diff] [review] Patch v1: fix urlbar alignment Why is this negative margin there at all? We don't have it on Windows, as far as I know.
Attachment #8399482 - Flags: feedback-
I mean only the negative top margin, of course.
This will need uplifting if 477948 lands
(In reply to Dão Gottwald [:dao] from comment #2) > Why is this negative margin there at all? We don't have it on Windows, as > far as I know. Wow, f-, you must really be sure this is fundamentally invalid. To align the `urlbar-back-button-clip-path` correctly, I made sure that the `#urlbarwrapper` element moves down 5px - see http://mxr.mozilla.org/mozilla-central/source/browser/themes/linux/browser.css#930 - and this is different than Windows, even though the clip-path is shared between both OSes. I needed to do this, because the clip-path changed since bug 893661. To keep the `#urlbar` element at the correct position, I applied a negative margin of 4px to move it back. This is wrong, because the original margin defines 2px on top and bottom, so it needed to be 5px - 2px = 3px to compensate, not 4px. That's what this patch does.
(In reply to Mike de Boer [:mikedeboer] from comment #5) > (In reply to Dão Gottwald [:dao] from comment #2) > > Why is this negative margin there at all? We don't have it on Windows, as > > far as I know. > > Wow, f-, you must really be sure this is fundamentally invalid. Pretty sure it's the wrong approach, yes. Windows uses -moz-box-align:center for #urlbar-container, Linux uses -moz-box-align:stretch. If you want to share the clip path, you need to be consistent there. If the clip path still doesn't fit after making this consistent, then it probably shouldn't be shared between Windows and Linux.
You were spot on, re. the box-align discrepancy! So changing that yields a tad less undesired behavior, but I still had to correct the `#urlbar-wrapper` and `#urlbar` upward by one pixel to correct the clip-path. I'm kinda hoping you might find this acceptable or have an idea how to correct this last bit?
Stealing; this completely gets rid of the transient margin-top and reduces the default margin for the url and search bars to be in line with the Windows styling.
Comment on attachment 8399564 [details] [diff] [review] patch v3 Review of attachment 8399564 [details] [diff] [review]: ----------------------------------------------------------------- <hugs> this works :) Thanks Dão!
Attachment #8399564 - Flags: review?(mdeboer) → review+
Comment on attachment 8399564 [details] [diff] [review] patch v3 [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 477948 User impact if declined: This needs to be uplifted together with bug 477948, since it's a regression. Testing completed (on m-c, etc.): landed on m-c. Risk to taking this patch (and alternatives if risky): minor. String or IDL/UUID changes made by this patch: n/a.
Mike, Firefox 30 was marked as unaffected and you requested an uplift. Mistake in the tracking flags usage?
Sylvestre, I requested uplift for this one too, because it technically belongs together with the patches in bug 477948. But since these haven't been uplifted yet, I'm not sure how to set the tacking flags for this one... maybe better to use tracking-firefoxX+ ?
Flags: needinfo?(mdeboer) → needinfo?(sledru)
Affected for both should should be fine.
Verified that the url is correctly aligned on Firefox 29 beta 8 (20140414143035), latest Aurora 30.0a2 and latest Nightly 31.0a1 2014-04-15.
You need to log in before you can comment on or make changes to this bug.