524915 - Quoted comments can overflow the comment box

Status: RESOLVED FIXED

(Bugzilla :: User Interface, defect, P4)

Description

[Fred Wenzel [:wenzel]]

Look at bug 524809 comment 1: Long lines totally break the layout.

This is a one-liner to fix. Just add

overflow-x:auto;

to the .bz_comment CSS class in the bugzilla standard themes. The box will get a horizontal scroll bar if too wide. If you could apply this to Mozilla's bz instance too, that'd be terrific.

Comment 1

[Frédéric Buclin]

Is it legal to add this CSS rule on <pre>?

Comment 2

[Frédéric Buclin]

Forget the question. A <div> is needed anyway.

Comment 3

[Fred Wenzel [:wenzel]]

Yup, I think that's legal. If you apply it to the entire comment (.bz_comment), which is a div, the header lines will scroll with it, but I don't think that's a problem at all.

Also, I hear overflow:auto on pre is buggy in MSIE.

Comment 4

[Max Kanat-Alexander]

I actually don't want to add scrollbars--even though it's ugly, I think it's more convenient to have the whole line displayed.

Comment 5

[Max Kanat-Alexander]

Adding additional scrollbars, particularly horizontal scrollbars, isn't really going to help here. Having a horizontal scrollbar on the comment might make Bugzilla *look* prettier (because it keeps things in the comment box for Dusk) but I don't think it would improve usability (and would probably harm it, since right now I just use the browser's horizontal scrollbar, which is basically the same or better in terms of usability).

Comment 6

[Fred Wenzel [:wenzel]]

Fixing it in Dusk only would probably have been better for UX than not fixing it at all, but that's okay.

FWIW, here's a jetpack to fix it: http://fwenzel.github.com/jetpacks/bugzilla/comment-overflow/

Comment 7

[Max Kanat-Alexander]

(In reply to comment #6)
> Fixing it in Dusk only would probably have been better for UX than not fixing
> it at all, but that's okay.

  What's the UX advantage?

Comment 8

[Fred Wenzel [:wenzel]]

The advantage is that the app doesn't look broken when text overflows its box. And the entire page width isn't dictated by a single user-entered comment.

Another possibility would be CSS3: word-wrap: break-word. That'll line-break long lines, but I am guessing that is worse for the user who actually pasted these long lines for a reason.

Comment 9

[Frédéric Buclin]

I agree that a single long comment shouldn't affect the whole UI. I don't know if you tried adding "overflow-x: auto;", but it helps a lot in keeping the UI as desired, and you seldom have to read such long comments anyway.

Even if we don't follow the overflow-x idea, we should do something about it.

Comment 10

[Max Kanat-Alexander]

(In reply to comment #8)
> The advantage is that the app doesn't look broken when text overflows its box.

  Okay. But UX stands for "Usability Engineering". What's the usability advantage?

> Another possibility would be CSS3: word-wrap: break-word. That'll line-break
> long lines, but I am guessing that is worse for the user who actually pasted
> these long lines for a reason.

  It still might be something we want to do. We'll see.

Comment 11

[Tanner M. Young [:tmyoung]]

Bug 508604 is already open for CSS3 word-wrap solution (I'd be happy to close mine as a duplicate).  I have tried it on Bugs that have this problem, and it didn't seem to compromise the integrity of any of the comments, it only made them more readable.

Comment 14

[Chris B.]

Odd.....I don't remember anything overflowing with these comments before Version 4 of Bugzilla.  But, since this bug was reported 1.5 years ago, it must have been evident (and still appears with these new boxes).  Maybe it didn't appear in the bugs I viewed.  There are thousands of bugs that need to be resolved, including many old ones.

That gets me thinking.....

I understand that new bugs get posted daily and the more time Mozilla takes to clean out old bugs there is less time to write new code.  However.....does anyone at Mozilla ever go through the really old bugs (over two or three years old) and either mark them as invalid (new code prevents the problem over the years) or update their status?  I have seen bugs patched in the past couple weeks that were at least four or five years old and there are still MANY problems regarding Java Applets posted in 2006 or 2007.

Comment 16

[[e-mail]]

(In reply to comment #8)
> Another possibility would be CSS3: word-wrap: break-word. That'll line-break
> long lines, but I am guessing that is worse for the user who actually pasted
> these long lines for a reason.
I've actually deliberately prefixed long lines with a > to prevent wrap ;-)

Comment 17

[Max Kanat-Alexander]

I honestly suspect that this problem will be handled by our UI redesign for Bugzilla 5.0, when the comment box simply looks totally different.

Comment 18

[Acho Arnold (:bePolite)]

Attachment: Proposed Patch

(In reply to Max Kanat-Alexander from comment #17)
> I honestly suspect that this problem will be handled by our UI redesign for
> Bugzilla 5.0, when the comment box simply looks totally different.

Its been 3 years since the last comment and I wonder if this issue has already been addressed in the 5.0 redesign.
I would also love to work on this issue since it really makes UI nasty

This patch made with respect to comment 0

> This is a one-liner to fix. Just add
>
> overflow-x:auto;
>
> to the .bz_comment CSS class in the bugzilla standard themes. The box will
> get a horizontal scroll bar if too wide.

Comment 19

[:glob ✱]

Comment on attachment 8404199 [details] [diff] [review]
Proposed Patch

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

i would prefer to see just the quoted text get the horizontal scrollbar, instead of the whole comment.
you should be able to look at how this is implemented on BMO's sandstone skin.

Comment 20

[Acho Arnold (:bePolite)]

Attachment: Updated Patch

(In reply to Byron Jones ‹:glob› from comment #19)
> i would prefer to see just the quoted text get the horizontal scrollbar,

This patch gives just the quoted text the horizontal bar similar to what is found here at bmo

Comment 21

[:glob ✱]

Comment on attachment 8404510 [details] [diff] [review]
Updated Patch

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

r=glob

thanks!

Comment 22

[Dave Miller [:justdave]]

wow, drama over such a change. :-)  I like it with the scrollbar personally, don't need the rest of the page getting wider to compensate.

Comment 23

[Dave Miller [:justdave]]

glob: can you commit this?  I don't think bePolite has commit access.

Comment 24

[:glob ✱]

oops, sorry about that.

To ssh://[e-mail]/bugzilla/bugzilla.git
   66c3f7d..641fe22  master -> master

Comment 25

[Kent Rogers [:mrbball]]

Could someone help me understand what benefit the fix to this bug provided?
It was one of the first things that someone complained about when we upgraded
to 5.0 and as far as I can tell, it doesn't add much (if any) value.

If a bug has multiple comments with wide quoted text, there is now one
scrollbar shown for each instance instead of one scrollbar for the entire
window, and I don't see how that is necessarily better.  Without scrollbars,
the wide quoted text does not appear to adversely affect anything else on the
page (the Classic skin does not have comment "boxes"), and with the additional
scrollbars it is busier.  I think comment 5 in this bug was spot on.

This feature should have at least been added as a user setting so that it
could be disabled.  I have a patch that would do that.  Is that a change that
would be considered?  If so, could I attach it here?

Comment 26

[Frédéric Buclin]

(In reply to Kent Rogers [:mrbball] from comment #25)
> Could someone help me understand what benefit the fix to this bug provided?

This has been explained in the comments above.


> It was one of the first things that someone complained about when we upgraded

"someone" means one person. You will never reach 100% agreement, independently of the feature you implement.


> This feature should have at least been added as a user setting so that it
> could be disabled.  I have a patch that would do that.  Is that a change that
> would be considered?  If so, could I attach it here?

IMO, a user pref for this doesn't make sense. In all cases, the patch on this bug has already been submitted and 5.0 has already been released, so you should file a separate bug and attach your patch there, but I suspect it may be closed as wontfix.

Comment 27

[Kent Rogers [:mrbball]]

(In reply to Frédéric Buclin from comment #26)
> (In reply to Kent Rogers [:mrbball] from comment #25)
> > Could someone help me understand what benefit the fix to this bug provided?
> 
> This has been explained in the comments above.
> 
I see nothing above that explains it.  The bug says that "comments can overflow the
comment box" (Dusk skin only), but I don't see why that's a problem.  Why did that
need to be "fixed"?

The best reasoning I see in the comments are Max's arguments to NOT do this.  As of
comment 17, it sounded like this was not going to be implemented and then out of the
blue a patch was submitted and accepted.

This was a terrible idea.  It greatly *reduces* the usability of the UI.  Since only
quoted text gets the scroll bars, you can have many chunks of text in a comment that
alternate between having scroll bars and not having scroll bars.  Extremely
inconvenient and messy to try to read.  The first bug I pulled up at our site had
comments with up to 9 scroll bars!  This was not thought through.

Other options that should have been considered to help improve the situation:

- Only have one scroll bar per comment, rather than one per quoted section
- Adjust the size of the scroll window to match the browser window

But for God's sake, at least make it a user preference so it is optional!

> 
> > It was one of the first things that someone complained about when we upgraded
> 
> "someone" means one person. You will never reach 100% agreement,
> independently of the feature you implement.
> 
No, MANY people (at least a dozen) reacted very negatively within two days of
our 5.0 upgrade, and I agree with them.  I'm sure there will be more dismay
as people realize what happened.

I had a bug opened against our Bugzilla due to the appearance that the quoted
text is simply being truncated.  In the Classic skin, if you don't notice the
scroll bar, that's exactly what it looks like.

> 
> > This feature should have at least been added as a user setting so that it
> > could be disabled.  I have a patch that would do that.  Is that a change that
> > would be considered?  If so, could I attach it here?
> 
> IMO, a user pref for this doesn't make sense. In all cases, the patch on
> this bug has already been submitted and 5.0 has already been released, so
> you should file a separate bug and attach your patch there, but I suspect it
> may be closed as wontfix.
>
You didn't say why you think a user pref doesn't make sense, but I suspect you
are right about another bug being wontfix'd (and another patch being ignored,
as much of the code I've submitted has been).  I guess I'll just add it to our
local system or remove this feature completely.

I believe I've said this before, but I think it's the attitude displayed here
that is the main reason people do NOT contribute to Bugzilla and will eventually
lead to the demise of the project. :-(

I did my best to represent Bugzilla positively and to try to contribute, but
the effort to overcome attitude like this and the associated frustration were
huge obstacles.  Unfortunately (IMO), my company is moving on to another bug
tracking tool, so I guess I won't worry about it too much ...

Comment 28

[Frédéric Buclin]

(In reply to Kent Rogers [:mrbball] from comment #27)
> I see nothing above that explains it.

Comment 0 and comment 8 explain that the whole layout looks broken if a single comment is too wide. The links on the right become unavailable without having to scroll the whole page horizontally. This argument is valid and this is what this bug tried to fix.


> out of the blue a patch was submitted and accepted.

The patch has been written, reviewed and approved by 3 distinct people. At some point, someone has to take the decision to accept a patch or not, which is why the Bugzilla project has reviewers and approvers. Nobody is perfect, but this patch has been in the core code for 12 months, has been available in *5* releases before 5.0 final (4.5.5, 4.5.6, 5.0rc1, 5.0rc2, 5.0rc3) and we asked for feedback on our website:

"Development releases exist as previews of the features that the next major release of Bugzilla will contain. They also exist for testing purposes, to collect bug reports and feedback, so if you find a bug in this development release (or you don't like how some feature works) please tell us."

The sentence in parentheses is exactly what your comments above are about. In the last 12 months, we got *no* negative feedback about this feature. It then sounds reasonable that this feature is acceptable. Or are we supposed to guess what people think?


> This was a terrible idea.  It greatly *reduces* the usability of the UI. 

As said above, we waited for such a feedback since Bugzilla 4.5.5 which has been released exactly one year ago.


> But for God's sake, at least make it a user preference so it is optional!

As I said, I don't think a user preference makes sense. Either this fix makes total sense and there is no reason to let users enable/disable it, or it should be backed out (which is pretty trivial to do as this means removing only 2 lines from a CSS file. Not a big deal).


> No, MANY people (at least a dozen) reacted very negatively within two days of
> our 5.0 upgrade, and I agree with them.  I'm sure there will be more dismay
> as people realize what happened.

Such feedback would have been welcome *before* the final release of 5.0. That's the whole point of doing several development snapshots and release candidates before the final release. It would be dishonest to say that we try to hide new features.


> You didn't say why you think a user pref doesn't make sense

Now I did.


> but I suspect you are right about another bug being wontfix'd

Don't take me wrong. My reasoning here is that your bug would ask exactly the opposite of what this bug asked. And if we backout the feature (the "famous" 2 CSS lines), then a 3rd bug would probably be opened again by a 3rd user complaining that the layout is broken again due to some long lines in a comment, etc... I have not been involved in the review or approval process of this bug, but you can see that this fix has been accepted and committed, and it's hard to imagine that they will change their mind right now. But maybe I'm wrong.


> I believe I've said this before, but I think it's the attitude displayed here
> that is the main reason people do NOT contribute to Bugzilla and will
> eventually lead to the demise of the project. :-(

I don't see why my attitude hurts you. I have been very factual, but in no way negative.
- Did someone explain in this bug what the problem was? Yes, in comment 0 and comment 8.
- Has this patch been committed yet? Yes, and our policy is very clear: do not attach a new patch in this bug, it's too late. File a separate bug instead.
- Is it reasonable to assume that asking for a user preference will be rejected? For such a feature, very likely, yes.

As I said, Bugzilla has a team of core developers who review contributions and a team of leaders who approve (or reject) contributions which pass reviews. This way to triage contributions is not specific to the Bugzilla project. All projects do the same (some even ignore *all* external contributions). None blindly accepts all contributions, and yes some contributors will be unhappy to see their contribution rejected.

Sorry if you think I'm rude, this is not my intention.

Comment 29

[Kent Rogers [:mrbball]]

(In reply to Frédéric Buclin from comment #28)
>
> Such feedback would have been welcome *before* the final release of 5.0.
> That's the whole point of doing several development snapshots and release
> candidates before the final release. It would be dishonest to say that we
> try to hide new features.
> 
Sorry, this feature was not in the release notes and I had not installed any
of the development snapshots, so I was not aware of it.  I gave feedback as
soon as I could.  I certainly would have given it earlier had I been aware,
although I'm pretty sure it wouldn't have mattered anyway.

I think this feature should be removed and either left out completely or
made to work better.  I'm planning to remove it from my installation.

> Sorry if you think I'm rude, this is not my intention.
>
I do.

Comment 30

[Frédéric Buclin]

FYI, bug 688205 is being worked on to correctly wrap quoted text. Having seen several comments with an horizontal scrollbar I also think it's ugly. Personally, I would prefer to clip too long lines, and display them when hovering them. That's trivial to do. But then I guess some users will complain that it's not ideal either.