228752 - [FIX] Scrollbar appears and disappears on textarea

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect)

Description

[Rob Janssen]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6b) Gecko/20031210
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6b) Gecko/20031210

While typing text into a certain textarea, a horizontal scrollbar nervously
appears and disappears while words are wrapping.

Reproducible: Sometimes

Steps to Reproduce:
1. go to website http://webwereld.nl/
2. click on an article
3. click on "Reageer" at bottom of text
4. enter text in the textarea

Actual Results:  
At first, there is no scrollbar at all.
After some typing a vertical scrollbar appears.
When continuing to type words, a horizontal scrollbar appears when the bottom
right of the textarea is reached, and disappears again when the word wraps to
the next line.

Expected Results:  
No horizontal scrollbar in a textarea that does wordwrapping.

Modern theme.
Only tested on Linux.

Comment 1

[Mitch]

Works fine for me on 20031215 build.

Comment 2

[Mats Palmgren (inactive)]

Attachment: Screenshot

Comment 3

[Mats Palmgren (inactive)]

Bug occurs in 2003-12-12-09 trunk on Linux
Bug does not occur in Mozilla 1.6a on Linux

-> Form Controls

Comment 4

[Mats Palmgren (inactive)]

Attachment: Testcase

The problem only occurs when there is a vertical scrollbar present.
It does not seem to occur in Quirks mode.

Comment 5

[Mats Palmgren (inactive)]

Attachment: Same testcase in Quirks mode

Comment 6

[Mats Palmgren (inactive)]

Backing out the patch for bug 103293 fixes this bug.

Comment 7

[Mats Palmgren (inactive)]

Attachment: Patch rev. 1

This fixes the problem...

Comment 8

[Boris Zbarsky [:bzbarsky]]

I bet the problem is the <br> that composer tosses in at the ends of lines and
the code at
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/html/base/src/nsBRFrame.cpp&rev=1.45&mark=187#183
that makes <br> be one twip wide in full-standards mode.  Mats, does commenting
out that line in the BRFrame code also make this bug go away?

If so, the attached patch is the way to go if we can't change the <br> code (I
do so love 4-year-old "temporary" hacks); if it's used, comments should be added
at both places pointing to each other....

Comment 9

[Hixie (not reading bugmail)]

(I've been wanting to nuke the BRFrame (in strict mode) for ages. It should so 
not be needed (and could be replaced with the theoretically identical "br { 
content: '\A'; }" or "br:before { content: '\A'; }"). Unfortunately, as bz 
points out, Composer has all kinds of hacks with magical <br> elements all over 
the place. I really don't understand why they are correct, and think they should 
probably all be removed, but there we are: if you remove BRFrame, things break.)

Comment 10

[Mats Palmgren (inactive)]

Yes, removing that line in BRFrame also fixes the problem - nice catch Boris.
I will add comments and limit the patch to Full Standards mode.

Comment 11

[Mats Palmgren (inactive)]

Attachment: Patch rev. 2

Adding 1 twip seems to be enough to fix the problem.  Added comments at
both places and limited the fix to Full Standards mode.
I have tested it with different (monospace) fonts and sizes, but not run the
block regression tests.

Comment 12

[Mats Palmgren (inactive)]

Nominating for 1.6 since it seems like a low-risc patch to fix a regression.

Comment 13

[Rob Janssen]

I agree with a nomination for 1.6, especially when it also affects Windows.
(I am testing 1.6b at home on Linux, but I would not roll-out 1.6 at work, on
Windows, with this bug still present.  We use lots of forms that will be less
fun to use with the flipping scrollbar)

Comment 14

[Brendan Eich [:brendan]]

Comment on attachment 137597 [details] [diff] [review]
Patch rev. 2

Maybe dbaron can look if bz is busy.

/be

Comment 15

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 137597 [details] [diff] [review]
Patch rev. 2

Looks good to me if you omit this part:

>   // 4 pixels add this on as additional padding(internalPadding). But only do
>   // this if charMaxAdvance != charWidth; if they are equal, this is almost
>   // certainly a fixed-width font.
>+  float p2t;
>+  aPresContext->GetPixelsToTwips(&p2t);
>+  nscoord t = NSToCoordRound(p2t);
>   if (charWidth != charMaxAdvance) {
>-    float p2t;
>-    aPresContext->GetPixelsToTwips(&p2t);
>     nscoord internalPadding = PR_MAX(charMaxAdvance - NSToCoordRound(4 * p2t), 0);
>     // round to a multiple of p2t
>-    nscoord t = NSToCoordRound(p2t); 
>     nscoord rest = internalPadding % t; 
>     if (rest < t - rest) {
>       internalPadding -= rest;

although we should really try to remove some of these hacks...

Comment 16

[Mats Palmgren (inactive)]

Attachment: Patch rev. 3

Fixed according to last comment.

Comment 17

[Brendan Eich [:brendan]]

Comment on attachment 137617 [details] [diff] [review]
Patch rev. 3

Let's get this in for 1.6, ASAP.  Thanks,

/be

Comment 18

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in to trunk, 2003-12-17 22:06 -0800.