RESOLVED FIXED 158700
REGRESSION(r201177): Web Inspector: ContentBrowser navigation bar should fit on a single line
https://bugs.webkit.org/show_bug.cgi?id=158700
Summary REGRESSION(r201177): Web Inspector: ContentBrowser navigation bar should fit ...
Matt Baker
Reported 2016-06-13 11:30:02 PDT
Created attachment 281182 [details] [Image] wrapping * SUMMARY ContentBrowser navigation bar should fit on a single line. * STEPS TO REPRODUCE 1. Inspector > Timelines 2. Select Network timeline, navigation bar path grows 3. Reduce inspector window width to the minimum => "Show Details Sidebar" button wraps, controls become misaligned (see screenshot)
Attachments
[Image] wrapping (116.39 KB, image/png)
2016-06-13 11:30 PDT, Matt Baker
no flags
WIP (1.18 KB, patch)
2016-06-20 13:06 PDT, Nikita Vasilyev
nvasilyev: review-
nvasilyev: commit-queue-
[Animated GIF] An attempt to fix the bug (416.09 KB, image/gif)
2016-06-21 12:07 PDT, Nikita Vasilyev
no flags
[Animated GIF] --navigation-bar-height vs --navigation-bar-height - 1px (41.58 KB, image/gif)
2016-06-21 13:34 PDT, Nikita Vasilyev
no flags
Patch (1.31 KB, patch)
2016-06-21 13:36 PDT, Nikita Vasilyev
no flags
Radar WebKit Bug Importer
Comment 1 2016-06-13 11:30:32 PDT
Matt Baker
Comment 2 2016-06-17 14:40:43 PDT
*** Bug 158885 has been marked as a duplicate of this bug. ***
Matt Baker
Comment 3 2016-06-17 15:16:09 PDT
Nikita Vasilyev
Comment 4 2016-06-20 13:02:49 PDT
This is similar to bug 157950.
Nikita Vasilyev
Comment 5 2016-06-20 13:06:36 PDT
Created attachment 281669 [details] WIP This fixes the bug. I'm looking if there are any other areas affected and if there's a way to solve this without setting a fixed height.
Joseph Pecoraro
Comment 6 2016-06-20 13:20:22 PDT
Comment on attachment 281669 [details] WIP Curious, why is the height -1?
Nikita Vasilyev
Comment 7 2016-06-20 13:36:28 PDT
(In reply to comment #6) > Comment on attachment 281669 [details] > WIP > > Curious, why is the height -1? Good question. This is how it was before r201177. It slightly changes vertical alignment of items.
Nikita Vasilyev
Comment 8 2016-06-21 12:07:39 PDT
Created attachment 281766 [details] [Animated GIF] An attempt to fix the bug I tried fixing this without setting a fixed item height. I expected it to be easy, but so far I had no luck. 1. Adding `flex-wrap: nowrap` makes all the items to fit on a single line. Unfortunately, it also changes height of navigation bar items in the sidebar. 2. Adding `align-items: center` fixes the height issues. Unfortunately, it makes dividers invisible.
Nikita Vasilyev
Comment 9 2016-06-21 13:34:30 PDT
Created attachment 281769 [details] [Animated GIF] --navigation-bar-height vs --navigation-bar-height - 1px
Nikita Vasilyev
Comment 10 2016-06-21 13:36:34 PDT
Blaze Burg
Comment 11 2016-06-21 13:41:20 PDT
Comment on attachment 281770 [details] Patch I don't understand how the analysis in this bug leads to this fix. Do we know why the bar started wrapping? Why do items in the navigation bar need to be shorter by 1 pixel? Is there a bug in the manual layout that's caused by setting height: auto instead of an explicit size? I don't think fiddling with the flex-* properties will lead to a fix if the bug is in our manual layout. Similarly, adjusting by 1px seems like a brittle fix if we don't know why it's necessary.
Joseph Pecoraro
Comment 12 2016-06-21 13:42:31 PDT
Comment on attachment 281770 [details] Patch r=me, seems fine.
Nikita Vasilyev
Comment 13 2016-06-21 13:58:28 PDT
(In reply to comment #7) > (In reply to comment #6) > > Comment on attachment 281669 [details] > > WIP > > > > Curious, why is the height -1? > > Good question. This is how it was before r201177. > It slightly changes vertical alignment of items. Better answer: .navigation-bar is 29px tall, but it has a 1px border at the bottom. Since we use `box-sizing: border-box`, navigation bar items should be 28px tall, one pixel less than the navigation bar.
WebKit Commit Bot
Comment 14 2016-06-21 14:11:31 PDT
Comment on attachment 281770 [details] Patch Clearing flags on attachment: 281770 Committed r202294: <http://trac.webkit.org/changeset/202294>
WebKit Commit Bot
Comment 15 2016-06-21 14:11:36 PDT
All reviewed patches have been landed. Closing bug.
Nikita Vasilyev
Comment 16 2016-06-21 14:45:15 PDT
(In reply to comment #11) > Comment on attachment 281770 [details] > Patch > > I don't understand how the analysis in this bug leads to this fix. Do we > know why the bar started wrapping? Yes, as Matt mentioned, it regressed in r201177. Specifically, this change: http://trac.webkit.org/changeset/201177/trunk/Source/WebInspectorUI/UserInterface/Views/NavigationBar.css > Why do items in the navigation bar need > to be shorter by 1 pixel? Is there a bug in the manual layout that's caused > by setting height: auto instead of an explicit size? I don't think fiddling > with the flex-* properties will lead to a fix if the bug is in our manual > layout. I think this is just how flexbox works, even though I find it not very intuitive. > Similarly, adjusting by 1px seems like a brittle fix if we don't > know why it's necessary. See my comment above.
Nikita Vasilyev
Comment 17 2016-06-21 14:47:31 PDT
Comment on attachment 281770 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=281770&action=review > Source/WebInspectorUI/UserInterface/Views/ContentBrowser.css:37 > + height: calc(var(--navigation-bar-height) - 1px); I should have just used `height: 100%`.
Note You need to log in before you can comment on or make changes to this bug.