Closed
Bug 567062
Opened 15 years ago
Closed 14 years ago
Wrong number in "x more" recipients button for singleline case
Categories
(Thunderbird :: Message Reader UI, defect)
Thunderbird
Message Reader UI
Tracking
(thunderbird3.1 .3-fixed)
VERIFIED
FIXED
Thunderbird 3.3a1
Tracking | Status | |
---|---|---|
thunderbird3.1 | --- | .3-fixed |
People
(Reporter: allblue, Assigned: rsx11m.pub)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
rsx11m.pub
:
review+
rsx11m.pub
:
ui-review+
standard8
:
approval-thunderbird3.1.3+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.1; de; rv:1.9.2.3) Gecko/20100401 Firefox/3.6.3 ( .NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; de; rv:1.9.2.5pre) Gecko/20100430 Thunderbird/3.1b2
TB 3.1beta2 now shows more recipients than TB 3.0.x. Depending on window size there is a button "x more" in the header, which tells how many recipients cannot be displayed. The number is too less
Reproducible: Always
Steps to Reproduce:
1. Compose/Send an mail e.g. 16 recipients
2. Look at the mail in the Drafts or Sent folder
Actual Results:
Depending on windows size you see some recipients listed in the header part of the mail, e.g. 4 recipients. At the right you'll find a button to show also the other recipients, labelled with "x more". The number is always to less, e.g. ("11 more").
Expected Results:
The correct number
This should be a side effect of bug 565209. The width used by the individual items in the header list is underestimated by the current algorithm, thus it is assumed that more addresses are shown than actually present, consequently the number shown with the "more" too small.
I'd expect this to resolve once the patch there is checked in.
It obviously didn't make it into 3.1 RC1, but I'm hoping to get it in for RC2. The issue described here is independent from what bug 565209 was originally opened for, so your bug is technically not a duplicate (hence confirming). Having run RC1 with the patch there for a while, it resolves this bug as well. Once a patched nightly build is available, I'll ask you to test it with your setup to confirm that it's fixed, then we can close it.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Depends on: 565209
Hardware: x86 → All
Whiteboard: [likely fixed by bug 565209]
Version: unspecified → Trunk
This didn't make it for RC2, and in fact needs a few days of "baking" on trunk first before it will be considered for the 3.1 branch. Thus, you can either wait until a regular 3.1 nightly for testing, or try a trunk tinderbox build which has the bug 565209 fix in it already. It works fine for me, but better make a backup of your 3.1 profile before trying a 3.2a1pre build.
The link for download is http://ftp.mozilla.org/pub/mozilla.org/thunderbird/tinderbox-builds/comm-central-trunk-win32/1276071456/ (for Windows).
http://kb.mozillazine.org/Testing_pre-release_versions
Thanks. Unfortunately I can't see any success. Same behavior as before.
In this screenshot you can see a mail with 17 recipients, but the number "x more" is wrong (compare source code):
http://www.imagebanana.com/img/r4nbtxaa/Zwischenablage1.png
On more detail: Opening the mail at first time, there is written "11 more". When I change the folder and com back, there is written "12 more".
Thanks for the screen shot. I think I see two issues:
- There is an encoding error between "Tobi Example 16" and "Tobi Example 17"
where the comma comes after #17 rather than #16, and the same happens again
between #10 and #11, thus you'll have to swap the commas here to get the
correct count. Here, you would have n=15 now, two pairs count once only.
- The second issue is more subtle; the "Tobi Example 03" address appears to
be the last one in the line, then comes the comma. Based on the original
implementation of bug 550487, the "Tobi Example 04" address may be just
hiding underneath the "more" text. So, this is a borderline case where
bug 565209 comment #26 actually would have made a difference.
I'm wondering a bit why you get "11 more" (i.e., the 4th address is counted and sliding underneath the "more" button) one time and "12 more" (i.e., the third address is the sliding one and the fourth is now counted as a hidden address) when coming back. There are apparently a few pixels difference between building the list from scratch and retrieving it from the cached addresses, but I don't see how this could be happening in the code (the commaNodeWidth is counted in either case and should be a constant).
If you vary the window width a little bit to get out of this borderline case, do you get consistent display and "more" counts then?
I apologize about this mistake. In order to make the screenshot anonymous, I replaced subject, body and the original addresses with the "example.com" ones. I made a Copy&Paste mistake, so the commas were missing.
In TB 3.1beta2 and rc1 I dealed with the original mail (with all commas needed) and the problem was existing there.
In fact, now I added the missing commas, and with this new version (incl. your patch) it is working as it should do. As long as the last shown address is displayed fractionally, this address is not counted in "x more":
http://www.imagebanana.com/img/g10ys4ml/Zwischenablage1.png
Thank you again and sorry for inconvenience.
No problem, that's why it was a good idea to show the testcase you've used! :-)
> - The second issue is more subtle; the "Tobi Example 03" address appears to
> be the last one in the line, then comes the comma. Based on the original
> implementation of bug 550487, the "Tobi Example 04" address may be just
> hiding underneath the "more" text. So, this is a borderline case where
> bug 565209 comment #26 actually would have made a difference.
I could reproduce this effect though; varying the right border of the 3-pane window in a borderline case where (i) addresses are fully visible and the (i+1)th address can just occupy a tiny space counts as (i+1) addresses shown even if the first pixel only is actually visible (in the single-line case).
Bryan, how big of a deal do you think this is? I could require a minimum
number of pixels of the last address being showable in order to consider it displayed and counting towards the "more" number, but it appears to me that this is a fairly negligible borderline issue of the overall approach.
Assignee | ||
Comment 10•14 years ago
|
||
This is an incremental patch to attachment 447859 [details] [diff] [review] introducing a
threshold to the last address in single-line mode. The following cases are distinguished:
- if at least 30px remain visible of the last address, it will slide under
the "more" button and not counted as hidden;
- if less than 30px would be visible of the last address, it is hidden and
counted in the "more" button.
The UX motivation for not showing the address <30px at all is to provide an unambiguous feedback to the user whether or not the address is included in the "more" button count (i.e., even if just partially visible, it is not). This also reduces code complexity. The 30px value is arbitrary but looks ok. Basing it on percentage width of the last address would cause variable appearance.
This works fine and introduces only a bit of additional code. If you think it's not worth it, just minus the patch and I'll close the bug given that everything else is resolved; if you like that modification, I'll ask Blake to review the patch with target 3.1.x.
Attachment #450362 -
Flags: ui-review?(clarkbw)
Assignee | ||
Comment 11•14 years ago
|
||
Steps for testing the patch, mailnews.headers.show_n_lines_before_more=1:
1. Find an e-mail with more recipients than fit into a single header line;
2. drag the right window border so that less than 30 pixel between a comma
and the "more" button remain (below-threshold case);
3. go to a different message and come back (due to bug 560698);
4. now you should see a space between the comma and the "x more";
5. drag the right border 30 pixels to the right (>threshold);
6. go to a different message and come back again;
7. now you should see part of the address and "x-1 more".
Updated•14 years ago
|
Attachment #450362 -
Flags: ui-review?(clarkbw) → ui-review?(nisses.mail)
Comment 12•14 years ago
|
||
Comment on attachment 450362 [details] [diff] [review]
Follow-up to bug 565209
Bryan wanted me to take over this ui-review
Assignee | ||
Comment 13•14 years ago
|
||
Thanks, any ETA for the review? It seems to be the only one in your queue.
Comment 14•14 years ago
|
||
Comment on attachment 450362 [details] [diff] [review]
Follow-up to bug 565209
Sorry for the delay, I had problems reproducing the bug properly up until now.
Even though we could probably do even better here (it still lies to me if I resize the window without switching between messages, but that can be another bug). Anyway, this seems to work better than previously, and I can't spot any drawbacks so ui-r+ from me.
Attachment #450362 -
Flags: ui-review?(nisses.mail) → ui-review+
Assignee: nobody → rsx11m.pub
Status: NEW → ASSIGNED
Summary: TB 3.1beta2: Wrong number in "x more" recipients button → Wrong number in "x more" recipients button for singleline case
Whiteboard: [likely fixed by bug 565209]
Assignee | ||
Comment 15•14 years ago
|
||
Comment on attachment 450362 [details] [diff] [review]
Follow-up to bug 565209
No problem. Resizing the window is bug 560698; I've noticed that too, but it behaves fine as long as you keep the window size intact.
Blake, I don't think there is any way to test this automatically as it depends on too many variables (including addresses, fonts, and window sizes), but the list in comment #11 should serve for a litmus test.
Attachment #450362 -
Flags: review?(bwinton)
Updated•14 years ago
|
Attachment #450362 -
Flags: review?(bwinton) → review+
Comment 16•14 years ago
|
||
Comment on attachment 450362 [details] [diff] [review]
Follow-up to bug 565209
>+++ b/mail/base/content/mailWidgets.xml Thu Jun 10 10:42:21 2010 -0500
>@@ -380,27 +380,31 @@
> // hide the last node spanning into the additional line (n>1)
>- if (curLine >= this.maxLinesBeforeMore && this.maxLinesBeforeMore > 1) {
>+ // also hide it if <30px left after sliding the address (n=1)
>+ if (curLine >= this.maxLinesBeforeMore &&
>+ (this.maxLinesBeforeMore > 1 || newLineWidth - overLineWidth < 30)) {
I think you could break this after the || and keep it under 80 characters.
Apart from that, I think I like it.
Later,
Blake.
Assignee | ||
Comment 17•14 years ago
|
||
Line-wrapping/white-space changes only; ui-r=andreasn, r=bwinton.
Attachment #450362 -
Attachment is obsolete: true
Attachment #452826 -
Flags: ui-review+
Attachment #452826 -
Flags: review+
Assignee | ||
Comment 18•14 years ago
|
||
Thanks for the reviews, push on comm-central please. I'll request branch approval once this landed on trunk without any problems.
Keywords: checkin-needed
Whiteboard: [c-n: comm-central]
Comment 19•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [c-n: comm-central]
Target Milestone: --- → Thunderbird 3.2a1
Assignee | ||
Comment 20•14 years ago
|
||
Comment on attachment 452826 [details] [diff] [review]
Patch for checkin
This works fine on today's trunk nightly, no test is throwing any errors.
Nominating for 3.1.1, it's a minor follow-up to bug 550487 and bug 565209 with minimum risk. Note that attachment 447859 [details] [diff] [review] needs to land first.
Attachment #452826 -
Flags: approval-thunderbird3.1.1?
Comment 21•14 years ago
|
||
Comment on attachment 452826 [details] [diff] [review]
Patch for checkin
As bug 565209 has been delayed to 3.1.2, I'm delaying this one as well.
Attachment #452826 -
Flags: approval-thunderbird3.1.1? → approval-thunderbird3.1.2?
Comment 22•14 years ago
|
||
Comment on attachment 452826 [details] [diff] [review]
Patch for checkin
a=Standard8 for 3.1.3. Please land on comm-1.9.2 default only.
Attachment #452826 -
Flags: approval-thunderbird3.1.2? → approval-thunderbird3.1.3+
Updated•14 years ago
|
Keywords: checkin-needed
Whiteboard: [needs bug 565209 landing on comm-1.9.2] → [cn to c192 default after/at same time as bug 565209]
Comment 23•14 years ago
|
||
Checked in to 1.9.2: http://hg.mozilla.org/releases/comm-1.9.2/rev/dfa775337b79
status-thunderbird3.1:
--- → .3-fixed
Keywords: checkin-needed
Whiteboard: [cn to c192 default after/at same time as bug 565209]
Flags: in-litmus?
Whiteboard: [see comment #11 for litmus-test steps]
Comment 24•14 years ago
|
||
Flags: in-litmus? → in-litmus+
Assignee | ||
Comment 25•14 years ago
|
||
Thanks Ludo, maybe add a remark that this applies for 3.1.3 and later only.
The "3.0 branch" category is potentially confusing (no "3.1 branch" there?).
Whiteboard: [see comment #11 for litmus-test steps]
Comment 26•14 years ago
|
||
(In reply to comment #25)
> Thanks Ludo, maybe add a remark that this applies for 3.1.3 and later only.
> The "3.0 branch" category is potentially confusing (no "3.1 branch" there?).
Well formal testing the big next round will be for 3.2.next, so I'm not worried about that.
Good question , I don't know if it's worth the effort yet to clone and create a new "branch" for test results.
Comment 27•14 years ago
|
||
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.9) Gecko/20100825 Thunderbird/3.1.3
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•