Closed
Bug 1400165
Opened 7 years ago
Closed 7 years ago
Margin around the search field in the bookmark sidebar should be bigger
Categories
(Firefox :: Theme, defect, P1)
Firefox
Theme
Tracking
()
VERIFIED
FIXED
Firefox 58
Tracking | Status | |
---|---|---|
firefox56 | --- | unaffected |
firefox57 | --- | verified |
firefox58 | --- | verified |
People
(Reporter: valentina.ona, Assigned: nhnt11)
References
(Blocks 1 open bug)
Details
(Whiteboard: [reserve-photon-visual] )
Attachments
(3 files)
1.59 MB,
image/bmp
|
Details | |
59 bytes,
text/x-review-board-request
|
dao
:
review+
Sylvestre
:
approval-mozilla-beta+
|
Details |
3.01 MB,
image/bmp
|
Details |
Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0 Build ID 20170914100122 [Affected version]: Nightly 57.0a1 [Affected platforms]: Windows 10 x64, Windows 10 x64 Surface Pro4, Ubuntu 16.04 x64, Mac OS x 10.12 [Steps to reproduce]: 1. Launch Nightly 57.0a1 2. Enable the Bookmark sidebar from the menu bar. [Expected result]: The distance between search box and bookmark box should be match the specifications. [Actual result]: The distance next to search box from bookmark side is smaller than spec. For more information please check the attachment.
Reporter | ||
Updated•7 years ago
|
Blocks: photon-visual
Whiteboard: [photon-visual][triage]
Comment 1•7 years ago
|
||
Nihanth, could you please evaluate this bug?
Flags: needinfo?(nhnt11)
Summary: the distance next to search box from bookmark sidebar is smaller than spec. → Margin around the search field in the bookmark sidebar should be bigger
Updated•7 years ago
|
Priority: -- → P3
Updated•7 years ago
|
Priority: P3 → P4
Comment hidden (mozreview-request) |
Assignee | ||
Comment 3•7 years ago
|
||
To be honest, I don't remember why I didn't match the spec on this, oh well. Anyway, for the default width of the sidebar, increasing the padding results in the history search bar being too small for the placeholder text on macOS, and overall feels very cramped, so I want to increase the default sidebar width if we're going to add this padding.
Flags: needinfo?(nhnt11)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → nhnt11
Status: NEW → ASSIGNED
Assignee | ||
Updated•7 years ago
|
Whiteboard: [photon-visual][triage] → [reserve-photon-visual]
Assignee | ||
Updated•7 years ago
|
Priority: P4 → P1
Updated•7 years ago
|
Iteration: --- → 57.3 - Sep 19
Flags: qe-verify?
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8910546 [details] Bug 1400165 - Increase padding of sidebar search container, adjust sidebar font-size on MacOS. https://reviewboard.mozilla.org/r/181992/#review187488 ::: browser/base/content/browser.xul:1189 (Diff revision 1) > # spacer should significantly out-flex the button. > <spacer flex="1000"/> > <toolbarbutton id="sidebar-close" class="close-icon tabbable" tooltiptext="&sidebarCloseButton.tooltip;" oncommand="SidebarUI.hide();"/> > </sidebarheader> > <browser id="sidebar" flex="1" autoscroll="false" disablehistory="true" disablefullscreen="true" > - style="min-width: 14em; width: 18em; max-width: 36em;" tooltip="aHTMLTooltip"/> > + style="min-width: 14em; width: 22em; max-width: 36em;" tooltip="aHTMLTooltip"/> (In reply to Nihanth Subramanya [:nhnt11] from comment #3) > Anyway, for the default width of the sidebar, increasing the padding results > in the history search bar being too small for the placeholder text on macOS, > and overall feels very cramped, so I want to increase the default sidebar > width if we're going to add this padding. Ugh. I think this is because we increase the font size in the side bar on Mac, but "18em" on #sidebar still refers to the default font size. Making the side bar wider across platforms doesn't seem like the right solution to that. Can we instead consistently increase the font size for #sidebar on Mac so that 18em refers to the right thing?
Attachment #8910546 -
Flags: review?(dao+bmo) → review-
Updated•7 years ago
|
Flags: qe-verify? → qe-verify+
Updated•7 years ago
|
QA Contact: valentina.ona
Updated•7 years ago
|
Iteration: 57.3 - Sep 19 → ---
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 7•7 years ago
|
||
mozreview-review |
Comment on attachment 8910546 [details] Bug 1400165 - Increase padding of sidebar search container, adjust sidebar font-size on MacOS. https://reviewboard.mozilla.org/r/181992/#review189706 ::: browser/themes/osx/places/places.css:60 (Diff revision 2) > #sidebar-search-label { > display: none; > } > > +#sidebar-search-container { > + padding: 4px; Why is this different on Mac vs. Windows/Linux?
Assignee | ||
Comment 8•7 years ago
|
||
(In reply to Dão Gottwald [::dao] from comment #7) > Comment on attachment 8910546 [details] > Bug 1400165 - Increase padding of sidebar search container, adjust sidebar > font-size on MacOS. > > https://reviewboard.mozilla.org/r/181992/#review189706 > > ::: browser/themes/osx/places/places.css:60 > (Diff revision 2) > > #sidebar-search-label { > > display: none; > > } > > > > +#sidebar-search-container { > > + padding: 4px; > > Why is this different on Mac vs. Windows/Linux? MacOS native searchbar styling already adds a 4px margin, also see bug 1382033 comment 12.
Comment hidden (mozreview-request) |
Comment 10•7 years ago
|
||
mozreview-review |
Comment on attachment 8910546 [details] Bug 1400165 - Increase padding of sidebar search container, adjust sidebar font-size on MacOS. https://reviewboard.mozilla.org/r/181992/#review189734
Attachment #8910546 -
Flags: review?(dao+bmo) → review+
Comment 11•7 years ago
|
||
Pushed by nhnt11@gmail.com: https://hg.mozilla.org/integration/autoland/rev/f63df3b6606a Increase padding of sidebar search container, adjust sidebar font-size on MacOS. r=dao
Assignee | ||
Comment 12•7 years ago
|
||
Comment on attachment 8910546 [details] Bug 1400165 - Increase padding of sidebar search container, adjust sidebar font-size on MacOS. Approval Request Comment [Feature/Bug causing the regression]: photon-visual polish [User impact if declined]: Search bar is a little cramped in the sidebar [Is this code covered by automated tests?]: no [Has the fix been verified in Nightly?]: not yet [Needs manual test from QE? If yes, steps to reproduce]: [List of other uplifts needed for the feature/fix]: [Is the change risky?]: low risk [Why is the change risky/not risky?]: minor CSS changes, avoiding saying "no risk" because there is a font-size adjustment, but any regression can easily be fixed and uplifted quickly [String changes made/needed]: none
Attachment #8910546 -
Flags: approval-mozilla-beta?
Comment 13•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f63df3b6606a
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox58:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Updated•7 years ago
|
status-firefox56:
--- → unaffected
status-firefox57:
--- → affected
Comment 14•7 years ago
|
||
Comment on attachment 8910546 [details] Bug 1400165 - Increase padding of sidebar search container, adjust sidebar font-size on MacOS. Polish photon, should be in 57b5
Attachment #8910546 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Reporter | ||
Comment 15•7 years ago
|
||
I tested this problem using Nightly 58.0a1 with the build ID 20171002100134 on Windows 10 x64 and Windows 7 x64 and I see no difference, but on Mac OS 10.12 and Ubuntu 16.04, the problem is resolved.
Flags: needinfo?(wkocher)
Comment 16•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/c7b47e1282c2
Flags: needinfo?(kwierso)
Assignee | ||
Comment 18•7 years ago
|
||
(In reply to Valentina Claudia Ona from comment #17) > Can you look on this issue? Filed bug 1411236. Thanks for spotting this.
Flags: needinfo?(nhnt11)
Reporter | ||
Comment 19•7 years ago
|
||
Based on the fact that has been filled a new bug 1411236, I will mark this as verified fixed.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in
before you can comment on or make changes to this bug.
Description
•