1198331 - Titles in sidebar aren't cropped and cross borders between tabs (titles are to long in some locales)

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[arni2033]

Attachment: 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

[arni2033]

Attachment: Possible solution B.png

Comment 2

[Patrick Brosset <:pbro>]

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

[arni2033]

(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

[arni2033]

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

[Raphaël Lustin [:rlustin] [:rlubitel] [:lubitel]]

Attachment: Titles in sidebar aren't cropped and cross borders between tabs (titles are to long in some locales).

Comment 6

[Raphaël Lustin [:rlustin] [:rlubitel] [:lubitel]]

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

[Raphaël Lustin [:rlustin] [:rlubitel] [:lubitel]]

Hi Jan,
Who can review this patch?
Thanks!

Comment 8

[Jan Keromnes [:janx]]

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

[Patrick Brosset <:pbro>]

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

[Raphaël Lustin [:rlustin] [:rlubitel] [:lubitel]]

Attachment: Titles in sidebar aren't cropped and cross borders between tabs (titles are to long in some locales).

Comment 11

[Raphaël Lustin [:rlustin] [:rlubitel] [:lubitel]]

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

[Patrick Brosset <:pbro>]

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

[Patrick Brosset <:pbro>]

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 14

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/6e82a342d591

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6e82a342d591