Last Comment Bug 803916 - filterHeader.label label in filterListDialog no longer wraps after bug 450302
: filterHeader.label label in filterListDialog no longer wraps after bug 450302
Status: RESOLVED FIXED
: polish
Product: Thunderbird
Classification: Client Software
Component: Filters (show other bugs)
: 17 Branch
: All All
: -- normal (vote)
: Thunderbird 19.0
Assigned To: :aceman
:
:
Mentors:
Depends on: 450302
Blocks:
  Show dependency treegraph
 
Reported: 2012-10-20 16:11 PDT by :aceman
Modified: 2012-10-29 03:04 PDT (History)
6 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed
fixed


Attachments
patch (1.19 KB, patch)
2012-10-20 16:16 PDT, :aceman
bwinton: ui‑review-
Details | Diff | Splinter Review
patch v2 (1001 bytes, patch)
2012-10-22 13:47 PDT, :aceman
mkmelin+mozilla: review+
acelists: ui‑review+
standard8: approval‑comm‑aurora+
standard8: approval‑comm‑beta+
Details | Diff | Splinter Review

Description :aceman 2012-10-20 16:11:38 PDT
In bug 450302 this change was made to the label containing &filterHeader.label; :
-        <label control="filterTree">&filterHeader.label;</label>
+        <hbox>
+          <label id="filterListLabel" control="filterList" value="&filterHeader.label;" crop="end" flex="1"></label>
+          <label id="countBox"/>
+        </hbox>

Thus, it no longer wraps if the dialog is too small. That is also exaggerated by the fact that the countBox label occupies the same grid cell and may force the filterHeader.label to be cropped if there are many filters (i.e. the countbox label has a long string).
Comment 1 :aceman 2012-10-20 16:16:46 PDT
Created attachment 673638 [details] [diff] [review]
patch
Comment 2 :aceman 2012-10-20 16:19:37 PDT
This may have a bigger impact on languages that have that string longer than the en-US locale.
Comment 3 Blake Winton (:bwinton) (:☕️) 2012-10-22 13:33:51 PDT
Comment on attachment 673638 [details] [diff] [review]
patch

I'm going to say ui-r-, because the current patch pushes the "3 items" out past the edge of the list, as https://dl.dropbox.com/u/2301433/Screenshots/ItemsShown.png shows.  (It should be where the "0 items" is, because it's the number of items in the list, not the number of buttons… ;)

So ui-r-, but feel free to give it a ui-r=me with that fixed.

Thanks,
Blake.
Comment 4 :aceman 2012-10-22 13:47:27 PDT
Created attachment 673993 [details] [diff] [review]
patch v2

OK, fixed that.
Comment 5 Magnus Melin 2012-10-27 12:05:43 PDT
Comment on attachment 673993 [details] [diff] [review]
patch v2

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

r=mkmelin
Comment 6 :aceman 2012-10-27 12:37:51 PDT
Archaeopteryx do you see this as a problem in any locale you can look at? Should it be nominated for TB17?
Comment 7 Sebastian Hengst [:aryx][:archaeopteryx] 2012-10-27 14:22:04 PDT
MXR shows the following translations (but don't trust mxr being up to date, see bug 803810):
http://mxr.mozilla.org/l10n-mozilla-aurora/search?string=filterHeader.label

Longest translation:
/nso/mail/chrome/messenger/FilterListDialog.dtd (View Hg log or Hg annotations)

    line 18 -- <!ENTITY filterHeader.label "Difilthara tše kgonagaditšwego di sepetšwa ka tsela ya othomethiki ka tatelano yeo e laetšwago ka fase.">

Gets shortened at: "[...] othomethiki..."

So maybe every fifth translation (I didn't count) translation doesn't show this by default. If you care about polish bugs, uplifting an idea is an option and makes localizers happy. If you don't want to change beta, it's in a resizeable dialog, so the user can work around this.
Comment 8 :aceman 2012-10-27 14:28:18 PDT
I think about nominating only because this got broken in TB17, it wrapped fine before. So if we fix it in TB17, it will never get exposed. If we do not fix it in 17, it will be an unnecessary regression until the next release a year after that.
Comment 9 Ryan VanderMeulen [:RyanVM] 2012-10-27 14:55:22 PDT
https://hg.mozilla.org/comm-central/rev/451f5e07043e
Comment 10 Mark Banner (:standard8) 2012-10-28 00:48:28 PDT
(In reply to :aceman from comment #8)
> I think about nominating only because this got broken in TB17, it wrapped
> fine before. So if we fix it in TB17, it will never get exposed. If we do
> not fix it in 17, it will be an unnecessary regression until the next
> release a year after that.

That's a very good reason for nominating.
Comment 11 :aceman 2012-10-28 14:06:03 PDT
Comment on attachment 673993 [details] [diff] [review]
patch v2

[Approval Request Comment]
Regression caused by (bug #): bug 450302
User impact if declined: comment 8
Testing completed (on c-c, etc.): 
Risk to taking this patch (and alternatives if risky): Should be none.

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