Last Comment Bug 1198331 - Titles in sidebar aren't cropped and cross borders between tabs (titles are to long in some locales)
: Titles in sidebar aren't cropped and cross borders between tabs (titles are t...
Status: RESOLVED FIXED
[good first bug][polish-backlog][diff...
:
Product: DevTools
Classification: Components
Component: Inspector (show other bugs)
: Trunk
: Unspecified Unspecified
-- trivial (vote)
: Firefox 44
Assigned To: Raphaël Lustin [:rlustin] [:rlubitel] [:lubitel]
:
: Gabriel [:gl] (ΦωΦ)
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2015-08-25 09:20 PDT by arni2033
Modified: 2018-06-13 10:27 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
affected
fixed


Attachments
Titles in sidebar aren't cropped and cross borders between tabs.png (13.56 KB, image/png)
2015-08-25 09:20 PDT, arni2033
no flags Details
Possible solution B.png (11.32 KB, image/png)
2015-08-25 09:20 PDT, arni2033
no flags Details
Titles in sidebar aren't cropped and cross borders between tabs (titles are to long in some locales). (1.10 KB, patch)
2015-09-24 13:05 PDT, Raphaël Lustin [:rlustin] [:rlubitel] [:lubitel]
no flags Details | Diff | Splinter Review
Titles in sidebar aren't cropped and cross borders between tabs (titles are to long in some locales). (2.72 KB, patch)
2015-09-28 15:25 PDT, Raphaël Lustin [:rlustin] [:rlubitel] [:lubitel]
pbro: review+
Details | Diff | Splinter Review

Description User image arni2033 2015-08-25 09:20:27 PDT
Created attachment 8652399 [details]
Titles in sidebar aren't cropped and cross borders between tabs.png

STR:   (or just see the screenshot)
1. Install Russian localization from https://www.mozilla.org/en-US/firefox/all/
2. Open Devtools-Inspector
3. Open sidebar in Inspector
4. Reduce width of the sidebar so that All Tabs dropmarker appeared bug you could still see part of Animation tab

Result:       Sidebar tab title "Блоковая модель" (Box model) crosses borders between tabs
Expectations: It should display "Блоковая м...", I guess.

Possible solutions:
A) Add [crop="end"] to <tab>s. That would make titles crop, just like Devtools tabs
B) Add "overflow:hidden;" for ".devtools-sidebar-tabs tabs > tab > .tab-middle"   (see screenshot "Possible solution B.png")

I prefer option (B), because I often change sidebar width, and text-overflow with "..." makes text twitch. The best solution here would be fade-in at the end of tab, as seen in Goggle chrome's tabs (not sure how to say that in English)
Comment 1 User image arni2033 2015-08-25 09:20:53 PDT
Created attachment 8652400 [details]
Possible solution B.png
Comment 2 User image Patrick Brosset <:pbro> 2015-08-31 05:24:00 PDT
Thanks for the report and for listing potential solutions.
I don't see a very big difference between A and B to be honest, and the advantage of A is that it'd be more consistent with how we deal with overflowing tab titles elsewhere in firefox. So I'd recommended this (also because it's a really simple fix).
Comment 3 User image arni2033 2015-08-31 05:35:46 PDT
(In reply to Patrick Brosset [:pbrosset] [:pbro] from comment #2)
> I don't see a very big difference between A and B to be honest
I only mentioned B because it displays more characters and doesn't cause text twitching on overflow.
Comment 4 User image arni2033 2015-09-06 13:33:37 PDT
Btw I think that the same should be applied to tabs in Debugger and Netmonitor (maybe in other places). That shouldn't be much harder. If somebody's going to work on this bug, ask Patrick about that.
Comment 5 User image Raphaël Lustin [:rlustin] [:rlubitel] [:lubitel] 2015-09-24 13:05:14 PDT
Created attachment 8665582 [details] [diff] [review]
Titles in sidebar aren't cropped and cross borders between tabs (titles are to long in some locales).
Comment 6 User image Raphaël Lustin [:rlustin] [:rlubitel] [:lubitel] 2015-09-24 13:11:53 PDT
Hi.

I've just attached a patch for this issue.
I set the attribute crop="end" to every tab added through the ToolSidebar.addTab() method.

Patrick, can you take a look? Thanks!
Comment 7 User image Raphaël Lustin [:rlustin] [:rlubitel] [:lubitel] 2015-09-26 03:28:21 PDT
Hi Jan,
Who can review this patch?
Thanks!
Comment 8 User image Jan Keromnes [:janx] 2015-09-28 03:04:33 PDT
Hi Raphaël! Thanks a lot for fixing this. I believe you correctly identified Patrick as a good reviewer for your patch, so I'll set the r? flag on him.
Comment 9 User image Patrick Brosset <:pbro> 2015-09-28 05:33:32 PDT
Comment on attachment 8665582 [details] [diff] [review]
Titles in sidebar aren't cropped and cross borders between tabs (titles are to long in some locales).

Review of attachment 8665582 [details] [diff] [review]:
-----------------------------------------------------------------

That's a nice fix because it's an sidebar.js which is a shared widget, so that will fix it everywhere it's used.
There's one detail though, this widget can be used with existing tabs, i.e. you don't have to use the widget's addTab method and in that case, that means that crop="end" attribute isn't going to be added. So we must add it where the other <tab>s are defined.

See /devtools/client/netmonitor/netmonitor.xul

This file defines a <tabbox id="event-details-pane"> element that contains a series of <tab> elements.
I believe you should add crop="end" to each one of them, so that we don't only use this bug to fix the problem in the inspector, but also in the netmonitor.

I've filed bug 1209023 to take care of the problem in the debugger, if you're interested in fixing another bug after this one, that's a good candidate.

So, I'm going to cancel the review for this patch, feel free to upload a new one with the fix in the netmonitor xul file, and ping me for review again.
Comment 10 User image Raphaël Lustin [:rlustin] [:rlubitel] [:lubitel] 2015-09-28 15:25:03 PDT
Created attachment 8666978 [details] [diff] [review]
Titles in sidebar aren't cropped and cross borders between tabs (titles are to long in some locales).
Comment 11 User image Raphaël Lustin [:rlustin] [:rlubitel] [:lubitel] 2015-09-28 15:28:06 PDT
Hi Patrick,
Thanks for your feedback. I've updated my patch accordingly. Can your review it please?
I'll take bug 1209023 to apply this in the debugger.
Comment 12 User image Patrick Brosset <:pbro> 2015-09-30 02:08:39 PDT
Comment on attachment 8666978 [details] [diff] [review]
Titles in sidebar aren't cropped and cross borders between tabs (titles are to long in some locales).

Review of attachment 8666978 [details] [diff] [review]:
-----------------------------------------------------------------

I like it. Thanks.
Try push so we make sure no tests fail because of this change: https://treeherder.mozilla.org/#/jobs?repo=try&revision=78ee068a1fd2
Comment 13 User image Patrick Brosset <:pbro> 2015-09-30 05:32:50 PDT
The try build hasn't completed, but given the nature of the change (css-only) and knowing that the tests have succeeded on some platforms already, we don't need to wait more to check this in.
Comment 15 User image Carsten Book [:Tomcat] 2015-10-02 03:55:16 PDT
https://hg.mozilla.org/mozilla-central/rev/6e82a342d591

Note You need to log in before you can comment on or make changes to this bug.