Closed Bug 77806 Opened 24 years ago Closed 4 years ago

Extremely long subject cause that body panel doesn't appear - sanity check subject length

Categories

(Thunderbird :: Message Reader UI, defect)

Product:
Thunderbird
Component:
Message Reader UI
Type:
defect
Priority:
Not set
Severity:
minor

Tracking

(thunderbird_esr78+ verified, thunderbird85 verified)

Status:
VERIFIED FIXED
Milestone:
86 Branch
Tracking Flags:
Tracking Status
thunderbird_esr78 + verified
thunderbird85 --- verified

People

(Reporter: ji, Assigned: mkmelin)

References

(Depends on 2 open bugs,
URL
)

Details

(Keywords: testcase, uiwanted, Whiteboard: [gs])

Attachments

(8 files)

Testcase 1: Message with JPN subject longer than 5000 chars
(deleted), message/rfc822
Details
Screenshot 1: Testcase 1 on TB 3.1
(deleted), image/png
Details
Testcase 2: Message with long subject (US-ASCII)
(deleted), message/rfc822
Details
Screenshot 2: Testcase 2 in "Vertical View" Layout of TB
(deleted), image/png
Details
Screenshot 3: Message 3 with long subject on TB 18 nightly, Linux
(deleted), image/png
Details
Screenshot 3b: Tooltip of long subject, message 3 (Tb18, Linux)
(deleted), image/png
Details
bug77806_subject_height.patch
(deleted), patch
Paenglab
: review+
BenB
: feedback+
wsmwk
: approval-comm-beta+
wsmwk
: approval-comm-esr78+
Details | Diff | Splinter Review
hover message list
(deleted), image/png
Details
****Observed with 04/26 trunk build***** Extremely long JPN subject shows as blank on the threadpane. Testcase will be attached later. Since the user seldom generates such a long subject, this is a very low priority bug.
Priority: -- → P5
Target Milestone: --- → Future
Status: NEW → ASSIGNED
Does this only applies to a JPN build?
warpozio@yahoo.com: please read comment 0, it clearly says "04/26 trunk build". you should be able to save the attachment to disk as a sibling of a mail folder with a name like "testfolder", run mail and select that folder (which should contain the single message). give it a try.
Product: MailNews → Core
Product: Core → MailNews Core
QA Contact: ji → i18n
This is strength result on Thunderbird 3.1. - If View - Headers - Normal is turned on, body text isn't appeared. - If View - Headers - All is turned on, it works fine. So I change summary and assigned to default.
Assignee: nhottanscp → nobody
Status: ASSIGNED → NEW
OS: Windows 95 → All
Hardware: x86 → All
Summary: Extremely long JPN subject shows as blank on the thread pane → Extremely long JPN subject cause that body panel doesn't appear
Attached image Screenshot 1: Testcase 1 on TB 3.1 (deleted) —
Component: Internationalization → Message Reader UI
Priority: P5 → --
Product: MailNews Core → Thunderbird
QA Contact: i18n → message-reader
Summary: Extremely long JPN subject cause that body panel doesn't appear → Extremely long subject cause that body panel doesn't appear - sanity check subject length
isn't just an intl issue
Target Milestone: Future → ---
Depends on: 525225
I wanted just to add "me too" (email from the Fedora update system) with clearly US-ASCII only email.
OT: attachment 558748 [details] also shows the waste of space on the right of the subject, due to the bad design of the header pane, where we reserve a full column-width for nothing except the date and a lost 'other actions' button somewhere far down.
We need an UI for overlong subjects, just truncating isn't sufficient. Perhaps this: 1a) truncate overlong subjects after more than x lines (where x = 5?) 1b) alternatively, but more difficult to implement, something along bug 511408 to auto-resize the header pane up to a certain percentage/user-defined limit 2) show (more of) full subject in tooltip (how do tooltips behave for insanely long strings? truncate tooltip if required) 3) have a "..." link at the end of truncated subject, clicking which will act like the "more..." button for to/cc: expand full subject, show scroll bars on header pane (and fix Bug 511408) Comments?
Whiteboard: uiwanted
URL: http://getsatisfaction.com/mozilla_me...
Whiteboard: uiwanted → [gs] uiwanted
Keywords: uiwanted
Whiteboard: [gs] uiwanted → [gs]
Just wanted to add another "me too". I got this message with a ridiculously long subject today, and here's what it looks like. Thomas D. asked about tooltips. Well, here you go... Not so useable either. I would suggest to simply truncate the value to some number for now, if there's no time to implement a better solution. Subjects like that aren't normal anyway, and even if anyone (a human, not a computer) uses them like that, then they should know better.
I'd say 200 characters should be enough for everyone. :)
Attachment #32356 - Attachment description: A mail with a JPN subject longer than 5000 chars. → Testcase 1: A mail with a JPN subject longer than 5000 chars.
Attachment #558747 - Attachment description: another example this time US-ASCII only → Testcase 2: another example, US-ASCII only
Attachment #32356 - Attachment mime type: text/plain → message/rfc822
Attachment #437195 - Attachment description: repro image on thunberbird 3.1 → Screenshot 1: Testcase 1 on TB 3.1
Attachment #32356 - Attachment description: Testcase 1: A mail with a JPN subject longer than 5000 chars. → Testcase 1: Message with JPN subject longer than 5000 chars
Attachment #558747 - Attachment description: Testcase 2: another example, US-ASCII only → Testcase 2: Message with long subject (US-ASCII)
Attachment #558748 - Attachment description: how does it look in the Thunderbird → Screenshot 2: Testcase 2 in "Vertical View" Layout of TB
Keywords: testcase
Attachment #664202 - Attachment description: Same problem on Thunderbird 18 nightly, Linux → Screenshot 3: Message with long subject on TB 18 nightly, Linux
Attachment #664202 - Attachment description: Screenshot 3: Message with long subject on TB 18 nightly, Linux → Screenshot 3: Message 3 with long subject on TB 18 nightly, Linux
Attachment #664204 - Attachment description: What tooltip looks like (Tb18, Linux) → Screenshot 3b: Tooltip of long subject, message 3 (Tb18, Linux)
Ben, Jim, can you comment why long subjects don't respect header pane maximum height and do not cause a scrollbar within header pane, as seen on Screenshot 1 of attachment 437195 [details] and Screenshot 2 of attachment 558748 [details]? I'd expect a similar behaviour as when clicking "3 more" button on long list of recipients (limited expanding of header pane, with scrollbar to see the remainder).
comment 15: > I'd expect a similar behaviour as when clicking "3 more" button on long list of recipients That was extremely complex to implement, and has performance implications, and subjects are expected to be no more than one line. comment 14: > I'd say 200 characters should be enough for everyone. :) Agreed. We should add a max-height: 2em; or so and an overflow-y: auto, but IIRC I tried that in another bug and it didn't work. Would be worth trying it again, in fact you can do it yourself in the live app using DOM Inspector.
(In reply to Ben Bucksch (:BenB) from comment #16) > comment 15: > > I'd expect a similar behaviour as when clicking "3 more" button on long list of recipients > > That was extremely complex to implement, and has performance implications, Sure. I'm not requesting the complexity of that feature in detail... > subjects are expected to be no more than one line. I think you yourself implemented Bug 489609 (subjectwrap) because many real-life subjects have more than one line. > comment 14: > > I'd say 200 characters should be enough for everyone. :) > Agreed. BMO bugs can have subjects with up to 255 characters, and I've seen a number of those in real life. Well, as long as we make the rest of the subject accessible, I guess 256 is a reasonable number; which amounts to something between 3 and 5 subject lines depending on screen size etc. > We should add a max-height: 2em; or so and an overflow-y: auto, The direction of that idea sounds good, perhaps a bit more or more flexible height. > but IIRC I > tried that in another bug and it didn't work. That's probably bug 489609. > Would be worth trying it > again, in fact you can do it yourself in the live app using DOM Inspector. https://addons.mozilla.org/de/thunderbird/addon/dom-inspector-6622/
Blocks: subjectwrap
A somewhat similar problem was fixed with a few lines in Bug 739311.
> We should add a max-height: 2em; or so and an overflow-y: auto, > but IIRC I tried that in another bug and it didn't work. Ah, indeed, due to bug 492645. Adding as dep. (This bug does not block 489609, though.)
No longer blocks: subjectwrap
Depends on: 492645
See bug 489609 comment 72 for explanation.
(In reply to Ben Bucksch (:BenB) from comment #19) > (This bug does not block 489609, though.) Ben, pls stop removing my changes to a bug without giving reason or asking for explanation. If you don't understand why other people are setting certain flags or dependencies, please ask and provide reasons that suggest removal, instead of briskly removing them without reason. It's not the first time, unfortunately. Explanation how bug 489609 "depends on" this bug - Short version: There is a clear and obvious relationship of code, UI/behaviour, and discussion of problems between bug 489609 and this bug. - Long version: For a start, it is common practice on BMO to link followup bugs (this bug) using the "Depends on" field of the main bug (bug 489609), regardless whether a strict technical dependency is present or not. In bug 489609, Ben helpfully introduced subject wrapping. Subject wrapping breaks for long subject (this bug). So this bug is an edge case of bug 489609 which isn't solved yet, hence the dependency. So if we want to fix this bug, it's certainly useful and necessary to look into exactly the same code that bug 489609 was touching. It doesn't matter if bug 489609 is technically responsible for this problem or not, and this is not about playing blame games. It's about providing helpful links to related code, technical information and discussion of bug 489609 that can help solve this bug. It's not helpful to remove such links/dependencies, because that will make it harder for everyone to find them somewhere in a long row of comments. And fwiw, using "See also" field would not be a good alternative either because it comes with a lot of informative and behaviour shortcomings (no tooltips, non-reciprocity of link, no friendly links, etc.) which makes it very inferior compared to a proper dependency link. > > We should add a max-height: 2em; or so and an overflow-y: auto, > > but IIRC I tried that in another bug and it didn't work. > Ah, indeed, due to bug 492645. Adding as dep. Bug 492645 is *one* possible explanation why bug 489609 was not able to address edge cases of long subjects. There might be other explanations and/or solutions.
Blocks: subjectwrap
> Ben, pls stop removing my changes to a bug without giving reason or asking for explanation. > If you don't understand why other people are setting certain flags or dependencies, > please ask and provide reasons that suggest removal, instead of briskly removing them > without reason. It's not the first time, unfortunately. Thomas, while we appreciate your help and effort, devs know more about the code, and make the decisions. I have all the rights to override you. A blocker is something that prevents a fix, something that needs to be done before it can be fixed. That this bug doesn't block bug 489609 is obvious, because bug 489609 is already fixed. > Bug 492645 is *one* possible explanation why bug 489609 was not able to address edge cases > of long subjects. There might be other explanations and/or solutions. No, it's *the* reason. I tried to fix this bug here as part of bug 489609, but couldn't due to bug 492645. I have actually tried it, I diagnosed it, and found the reason, and recorded it, as you can read there. I am getting very tired about you *constantly* contradicting me as a dev. Every bug ends up being a fight. Thus, this shall be my last comment about this matter here.
No longer blocks: subjectwrap
(In reply to Ben Bucksch (:BenB) from comment #22) > > Ben, pls stop removing my changes to a bug without giving reason or asking for explanation. > > If you don't understand why other people are setting certain flags or dependencies, > > please ask and provide reasons that suggest removal, instead of briskly removing them > > without reason. It's not the first time, unfortunately. > > Thomas, while we appreciate your help and effort, devs know more about the > code, and make the decisions. I have all the rights to override you. This isn't only or primarily about code, and I never interfered with anything code-wise here. I have not questioned your coding skills. This is about bug management, workflows, QA and providing/linking of information which is helpful and required for solving a bug. I don't think you have any more rights in that respect than I do, after more than 5 years of me contributing on BMO. Afasik, you're a volunteer contributor just like me. Even if you had "all the rights to override me" - to have rights and to use them wisely and cooperatively are two different stories... > A blocker is something that prevents a fix, something that needs to be done > before it can be fixed. That this bug doesn't block bug 489609 is obvious, > because bug 489609 is already fixed. No, it's not obvious at all. Between black and white, there's always shades of gray. We have plenty of bugs that are fixed and still depend on "blockers" which are still open, like Bug 545955 (quickfilterbar) which currently has 5 open blocking bugs listed in "depends on". You are right, a blocker is something that prevents a (full) fix, something that needs to be done before it can be (fully) fixed. In that sense, Bug 489609 isn't *fully* fixed, because subject wrapping clearly fails for the edge case of this bug. Or, in your own words from comment 22: > I tried to fix this bug here as part of bug 489609 > but couldn't due to bug 492645. So this bug apparently *should* have been fixed as *part* of bug 489609, but according to you, that wasn't possible because of Bug 492645. Unfixed parts of a bug surely qualify as a "blocker", if only to provide a direct and obvious link to such unfixed parts (this bug) from the main bug (bug 489609). As mentioned in my comment 21, most of which you either didn't read or deliberately ignored (nor answered appropriately), it is common practice on BMO to list related problems in the "depends on" field of their parent bugs, to create visible links between bugs, which does not always require a strictly technical relationship. Unfortunately, we only have a limited set of bug relationships in Bugzilla, so we need to use them with some flexibility. > > Bug 492645 is *one* possible explanation why bug 489609 was not able to address edge cases > > of long subjects. There might be other explanations and/or solutions. > No, it's *the* reason. I tried to fix this bug here as part of bug 489609, > but couldn't due to bug 492645. I have actually tried it, I diagnosed it, > and found the reason, and recorded it, as you can read there. I don't doubt that you've tried and done all that well. I do doubt that you never make mistakes. Perhaps the approach you tried to get this working is not the only approach that can be taken. E.g., you mention in Bug 492645 Comment 12 that another approach proven successful for similar problems might be to use the XUL "flex" attribute as a workaround to bug 492645. I don't know enough about the code yet to judge those approaches, but by setting the dependency, I am trying to open the door for others to look at the code in question (your patch in bug 489609) and perhaps come up with new ideas how to solve this problem. For that reason alone (and not to fight you personally), I'll re-add the dependency. By removing the dependency, you are actively and deliberately preventing progress on this bug. You're also adding to the impression that you don't want "your" bugs to come under scrutiny. > I am getting very tired about you *constantly* contradicting me as a dev. > Every bug ends up being a fight. Thus, this shall be my last comment about > this matter here. What, because you're a dev, it's not allowed to differ in opinion? That's a strange sense of cooperation, especially now that Mozilla withdraws most professional manpower from the TB project and we have to organize ourselves as a community of volunteers.
Re-adding dependency for ease of tracking, as explained in comment 23.
Blocks: subjectwrap
Well, at least the constant change of the bug relationships brought some attention to this bug by anybody watching the other bug given the load of e-mail notifications (and that would be the only kind of dependency which is justified as this bug technically isn't a regression of bug 489609, though the issue was revived with that fix). Ben is certainly correct that bug 492645 is the underlying cause that anything saying "no more than n lines for a wrapped box" fails on the CSS level. As I recall we had figured that XUL limitation out already during the bug 456596/bug 550487 work, which resulted in an explicit element-length determination and length counting to end up with an n-line address list and the subsequent "x more" button. I don't think that a similar approach is advised here, thus applying a maximum subject length (with the full subject shown in the View > Headers > All setting) as a constant or a function of the current message-viewer/window size might be a good compromise for the time being. In the end, fixing bug 492645 (and then bug 511408) would give a more thorough solution here much better options.
> anything saying "no more than n lines for a wrapped box" fails on the CSS level Even more so, "no more than n px for a wrapped XUL box" fails, too.
Thunderbird 15.0.1 is affected. I routinely get mails with very long subjects due to scripted output. Not only does the body not show in the index / pane view, but double clicking the mail to show it in a new tab still does not give the body, or a scroll bar to see it. Since this is effectively makes it impossible to read some mails, I consider this pretty serious.
Related bug for *sending* overlong subjects: Bug 192562
See Also: → https://bugzilla.mozilla.org/show_bug.cgi?id=192562
I just ran into this problem on thunderbird 17.0.5 (fedora 18)
same problem, Thunderbird 24 (Fedora 19).
Tried using the forward-button to get the data in a message-compose-view to at least somehow be able to read it. That just caused the CPU to spike though - Thunderbird not usable anymore. *wtf*
still an issue in 38.1.0 - /me wonders if an adversary could abuse this issue to bother TB users ?
This is still an issue right now, in version 52.1.1. Since it makes it impossible to read such an email, I think this bug should be fixed soon.

Looks like this would need css line-clamp (bug 866102)

Oh, which is implemented!

Attached patch bug77806_subject_height.patch (deleted) — — Splinter Review

Limit subject to max 3 lines, after which you get scrollbars instead.
Use https://bugzilla.mozilla.org/attachment.cgi?id=32356 to easily reproduce.

Assignee: nobody → mkmelin+mozilla
Status: NEW → ASSIGNED
Attachment #9194399 - Flags: review?(richard.marti)
Target Milestone: --- → 86 Branch
Attached image hover message list (deleted) —

Affects hovering over message list, but I'm not certain there is anything here to fix.

Comment on attachment 9194399 [details] [diff] [review]
bug77806_subject_height.patch

Looks a bit weird using -webkit- but there are no alternatives yet. Thanks.

Attachment #9194399 - Flags: review?(richard.marti) → review+

Comment on attachment 9194399 [details] [diff] [review]
bug77806_subject_height.patch

Good approach and idea to use CSS only

I concur with Richard. At least the "box" should have a standard or -moz alternative, leaving only -webkit-line-clamp with webkit prefix. Can you please try whether these work together, and if so, use them?

Attachment #9194399 - Flags: review-
Attachment #9194399 - Flags: review- → feedback+

(In reply to Ben Bucksch (:BenB) from comment #41)

Comment on attachment 9194399 [details] [diff] [review]
bug77806_subject_height.patch

Good approach and idea to use CSS only

I concur with Richard. At least the "box" should have a standard or -moz alternative, leaving only -webkit-line-clamp with webkit prefix. Can you please try whether these work together, and if so, use them?

I tried it, the -moz alternative doesn't work.

Thanks for testing this, to both of you.

Yes unfortunately standards line-clamp is not yet implemented, only -webkit-line-clamp, for compatibility purposes. And that requires display: -webkit-box;

(In reply to Wayne Mery (:wsmwk) from comment #39)

Created attachment 9194403 [details]
hover message list

Affects hovering over message list, but I'm not certain there is anything here to fix.

Imho the tooltip should also be cropped after a couple of lines.

That should be trivial, by limiting the tooltip to 500 characters:

   el.tooltip = msg.subject.substr(0, 500);

(You cannot limit the tooltip to a number of lines, because it's native code, and CSS is not applicable. You don't know how large the screen is. A limit to a number of characters is a good enough approximation. Just make sure that it's long enough so any remotely close to normal subjects are not cut, only insane ones like Wayne showed. The tooltip is useful to show subjects that are too long for a line - that's the whole reason why we have the tooltip in the first place: to show overly long subjects.)

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/260b9ca429e3
limit the subject to max 3 lines, for more than that use scrollbars. r=Paenglab

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED

This is the tooltip for a tree cell. I don't see exactly where we set it atm. Feel free to send a patch to fix it.

tracking-thunderbird_esr78: --- → +

Comment on attachment 9194399 [details] [diff] [review]
bug77806_subject_height.patch

[Approval Request Comment]
User impact if declined: for extremely long subjects, message body can't be seen
Testing completed (on c-c, etc.): c-c
Risk to taking this patch (and alternatives if risky): css only, not risky

Attachment #9194399 - Flags: approval-comm-esr78?
Attachment #9194399 - Flags: approval-comm-beta?

Comment on attachment 9194399 [details] [diff] [review]
bug77806_subject_height.patch

[Triage Comment]
Approved for beta

Attachment #9194399 - Flags: approval-comm-beta? → approval-comm-beta+

Used Testcase #2 in my testing of the 85.0b3 release candidate on Windows 10.

Saved the testcase.
Opened it and the Subject field had a scrollbar.

Opened a composition window and copied the subject and sent it to another account.
Opened the message and the Subject field had a scrollbar.
The tool tip showed the whole subject.

Comment on attachment 9194399 [details] [diff] [review]
bug77806_subject_height.patch

[Triage Comment]
Approved for esr78

Attachment #9194399 - Flags: approval-comm-esr78? → approval-comm-esr78+

Verifying in the 78.6.1 rc on Windows 10.

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: