Last Comment Bug 33654 - TEXTAREA incorrectly applying ROWS= and COLS=
: TEXTAREA incorrectly applying ROWS= and COLS=
Status: NEW
[html][behavior]relnote-devel
: testcase
Product: Core
Classification: Components
Component: Layout: Form Controls (show other bugs)
: Trunk
: x86 All
: P3 normal with 102 votes (vote)
: Future
Assigned To: Hideo Saito
:
: Jet Villegas (:jet)
Mentors:
http://bugzilla.mozilla.org/attachmen...
: 46106 98918 116779 133342 169265 174508 186271 193296 206946 214744 215099 234163 237523 244687 255038 255651 258272 261111 285228 291242 295352 304136 332451 343576 489598 633717 911226 (view as bug list)
Depends on: 292284
Blocks: 107086
  Show dependency treegraph
 
Reported: 2000-03-28 16:33 PST by Andrew McMillan
Modified: 2016-08-30 00:58 PDT (History)
87 users (show)
roc: blocking1.9.2-
roc: wanted1.9.2-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
example of using arial font (468 bytes, text/html)
2000-08-23 03:15 PDT, rods (gone)
no flags Details
Patch to use the correct font. (3.85 KB, patch)
2000-09-20 15:22 PDT, kinmoz
no flags Details | Diff | Splinter Review
Updated version of patch. r=sfraser@netscape.com (3.90 KB, patch)
2000-09-20 16:46 PDT, kinmoz
no flags Details | Diff | Splinter Review
Another testcase which doesn't specify font (897 bytes, text/html)
2000-09-29 04:16 PDT, Koike Kazuhiko
no flags Details
Interactive testcase (704 bytes, text/html)
2002-05-02 08:03 PDT, Pedro Lopes
no flags Details
http://www.cs.utah.edu/~newbold/mozilla.html (634 bytes, text/html)
2002-09-17 15:55 PDT, Mac
no flags Details
Rendering of <textarea rows=1> vs <input type=text > (ie vs moz) (1.93 KB, image/png)
2004-03-15 06:11 PST, Janick Bernet
no flags Details
Behaiuor in IE with cols=80 and 80 1s. The 1s go right up to the edge. (9.76 KB, image/png)
2004-04-05 08:19 PDT, rmjb
no flags Details
Behaviuor in Fx with cols=80 and 80 1s. There's space for 2 more 1s and the scroll bar only takes up 1 char. (10.17 KB, image/png)
2004-04-05 08:21 PDT, rmjb
no flags Details
patch (8.48 KB, patch)
2005-02-27 07:06 PST, Hideo Saito
no flags Details | Diff | Splinter Review
patch (9.22 KB, patch)
2005-02-27 07:55 PST, Hideo Saito
no flags Details | Diff | Splinter Review
patch for rows and cols (26.63 KB, patch)
2005-03-03 08:20 PST, Hideo Saito
no flags Details | Diff | Splinter Review
testcase for patch (369 bytes, text/html)
2005-03-03 08:37 PST, Hideo Saito
no flags Details
testcase for patch (389 bytes, text/html)
2005-03-03 08:43 PST, Hideo Saito
no flags Details
patch (27.36 KB, patch)
2005-03-06 08:03 PST, Hideo Saito
no flags Details | Diff | Splinter Review
screen shot for patch (61.24 KB, image/jpeg)
2005-03-06 08:10 PST, Hideo Saito
no flags Details
screen shot of appearance and disappearance of the scrollbars (55.40 KB, image/jpeg)
2005-03-15 08:26 PST, Hideo Saito
no flags Details
patch (23.22 KB, patch)
2005-03-20 08:52 PST, Hideo Saito
no flags Details | Diff | Splinter Review
testcase for overflow property (2.14 KB, text/html)
2005-03-20 09:00 PST, Hideo Saito
no flags Details
screen shot of the result applied the patch (52.34 KB, image/jpeg)
2005-03-20 09:07 PST, Hideo Saito
no flags Details
testcase for patch (843 bytes, text/html)
2005-12-16 17:17 PST, Hideo Saito
no flags Details
patch (21.25 KB, patch)
2005-12-16 17:21 PST, Hideo Saito
no flags Details | Diff | Splinter Review
patch (24.61 KB, patch)
2008-08-21 07:42 PDT, Hideo Saito
no flags Details | Diff | Splinter Review
patch (27.43 KB, patch)
2008-08-23 17:26 PDT, Hideo Saito
no flags Details | Diff | Splinter Review
patch (19.82 KB, patch)
2008-08-26 05:25 PDT, Hideo Saito
roc: review-
Details | Diff | Splinter Review
patch (18.16 KB, patch)
2010-04-05 21:11 PDT, Hideo Saito
no flags Details | Diff | Splinter Review
testcase for patch on wrap="off" (941 bytes, text/html)
2010-04-16 03:24 PDT, Hideo Saito
no flags Details
patch (18.28 KB, patch)
2010-04-16 03:38 PDT, Hideo Saito
no flags Details | Diff | Splinter Review
base patch (19.46 KB, patch)
2010-04-17 07:42 PDT, Hideo Saito
no flags Details | Diff | Splinter Review
additional patch for another bug (1.98 KB, patch)
2010-04-17 07:52 PDT, Hideo Saito
no flags Details | Diff | Splinter Review
base patch (18.13 KB, patch)
2010-04-18 17:59 PDT, Hideo Saito
no flags Details | Diff | Splinter Review
patch (20.16 KB, patch)
2010-05-01 09:15 PDT, Hideo Saito
no flags Details | Diff | Splinter Review
patch which does not remove space (17.74 KB, patch)
2010-05-06 18:10 PDT, Hideo Saito
no flags Details | Diff | Splinter Review
patch which does not remove space (25.64 KB, patch)
2010-05-16 18:28 PDT, Hideo Saito
no flags Details | Diff | Splinter Review

Description Andrew McMillan 2000-03-28 16:33:46 PST
See the URL for an example of a TEXTAREA field specified as ROWS=5 COLS=30 which
is being displayed as (approximately!) ROWS=7 COLS=48
Comment 1 rods (gone) 2000-03-29 12:30:45 PST
This looks like the input text or the textarea aren't getting the correct font.
Comment 2 Andrew McMillan 2000-03-29 12:41:02 PST
Well, TEXTAREA isn't getting the correct font (see
http://bugzilla.mozilla.org/show_bug.cgi?id=28219) but that's a different
problem, although possibly related.

But when I look at the page (Linux 2000032711 build) the INPUT TYPE=TEXT field
is shown in the font specified in the style, but it is waay too wide.  Similarly
for TEXTAREA, except that the correct fotn is not used, and the area is also too
tall, even when scroll bars appear.
Comment 3 rubydoo123 2000-04-03 13:38:38 PDT
assigning to Kin
Comment 4 kinmoz 2000-04-19 14:10:56 PDT
Accepting bug.
Comment 5 rubydoo123 2000-05-04 15:22:07 PDT
moving to m17
Comment 6 rubydoo123 2000-07-19 11:25:39 PDT
we are still getting the wrong information for laying out the textarea. There is 
additional spaces in the column and an extra row, this probably has something to 
do with preserving space for the scrollbars
Comment 7 Marc Attinasi 2000-07-21 15:22:09 PDT
*** Bug 46106 has been marked as a duplicate of this bug. ***
Comment 8 rubydoo123 2000-07-27 08:13:59 PDT
setting to nsbeta3+
Comment 9 ekrock's old account (dead) 2000-07-27 16:02:32 PDT
Adding brackets around nsbeta3+. (Tell me if we've dropped that convention.)
Comment 10 rubydoo123 2000-07-31 14:09:35 PDT
setting priority in status whiteboard
Comment 11 Hervé Renault 2000-08-22 05:23:13 PDT
i've read a post in an html authoring newsgroup about this problem. to
summarize, the (upset) poster tells that

if you have this in your stylesheet :
textarea { font-size: 8pt }

and this in your html form :
<textarea name="texte" cols="40" rows="12"></textarea>

then IE renders the correct textarea width, but Mozilla seems to compute the
width of the textarea based on a 12pt font (!)
if you type 40 characters, they do not add up to the full width of the area, as
you may have already noticed...
Comment 12 kinmoz 2000-08-22 17:21:53 PDT
per beppe bouncing back to rods@netscape.com.

Rod, we are using the nsFormControlHelper methods to get our font and calculate
the control size ... is this a bug in the FormControlHelper methods? Or are we
using it incorrectly? Help?
Comment 13 rods (gone) 2000-08-23 03:15:32 PDT
Created attachment 13337 [details]
example of using arial font
Comment 14 rods (gone) 2000-08-23 03:18:35 PDT
The sizing is working fine. Basically, if it sizes correctly with the default 
font, which it is, then the problem lies elsewhere.

The problem is the textarea is not using the correct font. I have just attached 
an example of the textarea using "arial" and it isn't displaying that font.

In the url field the example is using "tahoma" which is very close to arial and 
again it isn't using tahoma.

reassigning back to you.
Comment 15 kinmoz 2000-08-23 13:16:25 PDT
Accepting bug.
Comment 16 rubydoo123 2000-08-24 07:38:25 PDT
Kin will debug this one to find out why we are not getting the appropriate data 
from the control. Rod, Kin may need your help to fix whatever is broken, it may 
require making changes in your area, it will just be easier to have Kin fix it 
instead of continuing to bounce it back and forth
Comment 17 kinmoz 2000-09-11 13:55:17 PDT
Okay, so I see that the editor is forcing the content area of the TextControl to 
use "font-family: monospace;" when the editor's PlainText flag is set.

If I remove this code, the correct font is used ... though the example attatched 
to this bug still creates a 30 column textarea where the string 
'123456789012345678901234567890' (30 chars) takes up about 2/3 of the width in 
NavQuirks mode. This happens because the nsFormControlHelper method is 
calculating the width of the textarea by taking the average width of the 'w' and 
'W' characters in the font and multiplying it by the number of columns and 
adding the scrollbar width. Rod, is that how NavQuirks mode is supposed to work?
Comment 18 rubydoo123 2000-09-18 08:15:24 PDT
confirming priority
Comment 19 kinmoz 2000-09-20 15:22:51 PDT
Created attachment 15144 [details] [diff] [review]
Patch to use the correct font.
Comment 20 kinmoz 2000-09-20 15:25:13 PDT
I just attatched a patch that makes the editor use the correct font.

Cc'ing akkana@netscape.com for help in testing this patch since it touches code 
used by PlaintText MsgCompose and PlainText editor.
Comment 21 kinmoz 2000-09-20 16:46:05 PDT
Created attachment 15156 [details] [diff] [review]
Updated version of patch. r=sfraser@netscape.com
Comment 22 kinmoz 2000-09-20 17:20:29 PDT
I'm going on vacation so mjudge has graciously agreed to check this in for me. 
:-)
Comment 23 anthonyd 2000-09-21 14:07:12 PDT
as soon as tree opens i will check this in
-mjudge
Comment 24 mjudge 2000-09-25 12:02:30 PDT
fixed.
Comment 25 bsharma 2000-09-26 18:01:48 PDT
Updating QA contact.
Comment 26 Andrew McMillan 2000-09-26 18:14:32 PDT
Although this is now using the COLS=... attribute according to
rods@netscape.com's definition of 'correctly'.  The ROWS=... attribute still
seems to be working wrong.  In my example page the TEXTAREA is set to 5 rows,
but the display contains space for 8 rows (plus scroll bar), at least it is with
the Linux build from 20000925.
Comment 27 rubydoo123 2000-09-27 10:10:07 PDT
reassign to kin for evaluation
Comment 28 Phil Peterson 2000-09-27 22:30:47 PDT
Not holding PR3 for this. Marking nsbeta3- and nominating for rtm so we don't
lose track of the patch.
Comment 29 kinmoz 2000-09-28 11:37:14 PDT
Accepting bug.
Comment 30 Koike Kazuhiko 2000-09-29 04:15:04 PDT
I will attach another testcase which doesn't specify font.

Height and Width of textarea are not changed correctly after switching
character coding. Too small or too large in such a case.
Comment 31 Koike Kazuhiko 2000-09-29 04:16:02 PDT
Created attachment 15816 [details]
Another testcase which doesn't specify font
Comment 32 kinmoz 2000-09-29 11:25:06 PDT
Clearing the "Fix in hand" status. Beppe, I'll need this rtm+'d to work on this.
Comment 33 rubydoo123 2000-09-29 15:06:02 PDT
textareas are a highly used form element, this does not have a workaround, this 
should be fixed for rtm

Kin, please follow guidelines for rtm bugs
Comment 34 rubydoo123 2000-10-01 16:50:44 PDT
m19
Comment 35 rubydoo123 2000-10-09 11:24:24 PDT
removing the + per pdt sw rules
Comment 36 kinmoz 2000-10-12 17:31:05 PDT
I'm going to future this bug since the pdt committee probably won't let me check 
this in on the Netscape_20000922_BRANCH.

Andrew, I'm not seeing 8 rows when I display your page on my Linux box. I see it 
calculating 6 rows + scrollbar which rods@netscape.com's NavQuirks code says to 
calculate. If I switch viewer to use the Standard mode, I get 5 rows + a 
scrollbar.

As for the bug KOIKE Kazuhiko's sample points out, it's using the wrong font 
because in html.css, we tell all input elements and textareas to use the 'field' 
font:

    input { font: field; }

so that we get a monospace font by default in all input elements and textareas. 
To fix this bug, I'll have to add the following style to html.css:

    font input { font: inherit; }
    font textarea { font: inherit; }

But this may introduce some performance issues when rendering pages with input 
elements and textareas because of the search up the parent hierarchies that 
needs to be done when resloving the style.
Comment 37 rubydoo123 2000-10-17 09:55:48 PDT
removing need info and adding rtm-
Comment 38 kinmoz 2000-10-18 10:10:48 PDT
Changing milestone from "Future" to "mozilla0.9".

Adding relnotesRTM keyword.
Comment 39 Gervase Markham [:gerv] 2000-10-24 11:55:44 PDT
What's the exact issue left here, then? (for the release note)

Gerv
Comment 40 kinmoz 2000-10-26 07:52:59 PDT
For the release notes:

Form input elements and textareas will not use the font face specified by <FONT>
tags that enclose them. To get around this, the author of the web page can use
style to explicitly set the font used by input elements or textareas, or they
can add the following style to the <HEAD> portion of their document so that
input elements and textares use the face specified by the <FONT> tag:

    font input    { font: inherit; }
    font textarea { font: inherit; }

Comment 41 ekrock's old account (dead) 2001-01-18 13:14:34 PST
Nom. nsbeta1. b.c. and correctness bug we'd like to fix for nsbeta1.
Comment 42 Vladimir Ermakov 2001-03-06 16:55:14 PST
QA Contact Update
Comment 43 kinmoz 2001-03-26 14:59:25 PST
I'm going to close this bug as fixed, since the original reported bug was fixed. 

The fact that text widgets don't pick up the font specified in a <font> tag is 
something shared by all gfx form widgets, if a fix is made for quirk mode, as 
discussed in bug #53360, text widgets will get the fix for free.
Comment 44 Koike Kazuhiko 2001-03-27 15:14:45 PST
I can't believe this was fixed.

http://wrms.catalyst.net.nz/mozilla-bug-demo.php

The textarea in this page is displayed as rows=6 and column=51 on 2001032708/Linux.

Comment 45 kinmoz 2001-04-12 16:39:54 PDT
mozilla.0.9.1
Comment 46 rubydoo123 2001-05-24 11:12:15 PDT
when I created a plain textarea:
<textarea name="textarea test" rows="6" cols="30"></textarea>

I was able to type in the following data, without getting vertical or horizontal 
scroll bars -- and I should have once I exceeded 30 characters in width and/or 6 
 lines. Note, there are 32 characters in width -- when I add the 33rd character 
I get the horizontal scrollbar. Now comes the funky part -- if I delete the 33rd 
character, I get the horizonatal and vertical scrollbars (what is that all 
about?). 

For the rows issue, I am able to type in 7 rows without getting the vertical 
scrollbar, the scrollbar does not display. If I go to the last line (the # 7) 
and hit enter, I get both the vertical and horizontal scrollbars -- why? 
SHouldn't I just get the vertical scrollbar?

Here is a copy of the text I typed in:

12345678901234567890123456789012
2
3
4
5
6
7

adding glazman, maybe there is some odd CSS thing happening.

Does the textarea frame include the area reserved for the scrollbars? So, if you 
see an empty textarea, and note the width and height (frame) is that the overall 
width and height, or does that change when the scrollbars get displayed? Could 
that be the problem? Could the text being inserted actually roll into the 
reserved space for the scrollbars?
Comment 47 Daniel Glazman (:glazou) 2001-05-25 10:08:38 PDT
kin, you should probably take a look at bug 82715 which seems related to the
current one...
Comment 48 rubydoo123 2001-06-26 15:52:30 PDT
this can and will affect how pages are rendered

reviewed and approved
Comment 49 rubydoo123 2001-07-10 17:20:22 PDT
putting out to 1.0
Comment 50 n-baxley 2001-08-09 08:08:27 PDT
Here is another example using rows=1 where it obviously is not 1 row.  This was
done on Windows 2000 and it doesn't work as of 0.9.3.
Comment 51 n-baxley 2001-08-09 08:10:02 PDT
I mean here (http://www.baxleys.org/Mozilla/MozTextarea.htm) is another example.
Comment 52 puddy 2001-09-16 04:53:16 PDT
is 98918 a dup of this bug?
Comment 53 Vladimir Ermakov 2001-09-17 14:18:17 PDT
*** Bug 98918 has been marked as a duplicate of this bug. ***
Comment 54 Jeff Hughes 2001-09-21 14:48:45 PDT
http://linux.apptechsys.com/textbox.html is another example of how rows are not
properly displayed.
Comment 55 kinmoz 2001-11-27 15:25:23 PST
Bulk move of mozilla1.0 bugs to mozilla.1.0.1. I will try to pull some of these
back in if I can.
Comment 56 m_mozilla 2001-12-02 10:56:19 PST
Are any of these sufficiently simmilar to warrant
consolidation via duplication or dependancy change?

    bug 33654
    bug 52500
    bug 92980
    bug 103293
    bug 109392

-matt
Comment 57 Flo Ledermann 2002-01-20 11:45:11 PST
why is this bug not in 1.0?

a textarea specified as having 1 row rendered as 3 rows is a serious issue,
IMHO. I thought those were exactly the bugs that have to be cleared pre-1.0.
Comment 58 Kevin McCluskey (gone) 2002-02-05 17:25:38 PST
Moving to Mozilla 1.1. Engineers are overloaded with higher priority bugs.
Comment 59 rubydoo123 2002-03-08 11:59:48 PST
removing myself from the cc list
Comment 60 Tuukka Tolvanen (sp3000) 2002-03-26 03:19:18 PST
*** Bug 133342 has been marked as a duplicate of this bug. ***
Comment 61 a_geek 2002-03-30 04:14:16 PST
While laying out a form with <textarea> and <input type="text"> fields, I found
that the textarea is correctly displayed in monospace font while the other
<input type="text"> fields are displayed in proportional font. This just kills
any type of layout control for forms on different browsers w/o resorting to
major hacking. OTOH, the design of forms is mainly determined by function.
Throwing layout control into your web application programs doesn't make sense
(as does not oversizing fields only to make them look right).
Form fields should _always_ be displayed in monospace font, IMO.

This is on Linux, using Mozilla 2002022613.
Comment 62 Akkana Peck 2002-03-30 12:40:22 PST
Mac people lobbied for variable-width fonts in single-line text fields, and got
their way, over the objection of unix people (who didn't really fight that hard
-- no one felt particularly strongly against it).  There's a bug (long since
marked fixed) that tracked the discussion, but you'd have to search for it -- I
was cc'ed, I'm pretty sure sfraser was as well (or perhaps filed it), but I
can't remember what the summary was so you'll have to do some searching.

You should be able to override that and force fixed-width with an entry in your
userContents.css -- see http://www.mozilla.org/unix/customizing.html
Comment 63 Pedro Lopes 2002-05-02 08:03:11 PDT
Created attachment 82028 [details]
Interactive testcase

This bug does not depend on variable width fonts or the presence of font tags
(the testcase uses neither).
There is also no difference between quirks and standards mode (the testcase is
in standards mode)

The behaviour I get is this:
- there are always 3 more colums than specified
- there is always 1 more row than specified, except:
- when you ask for one row, you get 3

Some of the extra space migh be reserved for the scrollbars. Even if that is
the intended behaviour (is it?), it is reserving far too much space.
Comment 64 TucsonTester3 2002-05-07 14:54:50 PDT
Also see this in windows 2000.
Comment 65 Ko Young-rok 2002-06-10 08:34:12 PDT
I'm using win 2k & mozilla 1.0 {Build2002023012}.
following textarea code not viewed properly.
<textarea name=memo  cols=42  rows=20 class=input></textarea>
It's in URL of
http://sgcc.net/bbs/write.php?id=malmo90&page=1&page_num=10&category=
&sn=off&ss=off&sc=off&keyword=&select_arrange=headnum&desc=asc&no=
&mode=write
so long.. :)
Document was written in EUC-KR character coding.

It seemed that COLS is working propely. But 'rows=20' isn't work.
The row size in the mozilla screen is exatly '1'.

I try to fix my html codes and I found that downloaded document work properly.
(Downloaded document han no additional images, objects.. etc.
It's just text document that includes html, css, javascript codes.)
Comment 66 Mats Palmgren (:mats) 2002-09-17 15:35:23 PDT
*** Bug 169265 has been marked as a duplicate of this bug. ***
Comment 67 Mats Palmgren (:mats) 2002-09-17 15:47:30 PDT
This is now happening with no fonts specified in the markup at all.
Just a simple <textarea cols=40 rows=2></textarea> will display with 3 rows.
In 2002-09-17 Linux & win2000 trunk builds I get exactly N+1 rows when rows=N.
Comment 68 Mac 2002-09-17 15:55:05 PDT
Created attachment 99581 [details]
http://www.cs.utah.edu/~newbold/mozilla.html

Dead simple testcase. No frills, nothing fancy, just defaults.
Demonstrates 4 sizes of textareas, half of which are incorrect.
Comment 69 José Jeria 2002-10-15 03:11:59 PDT
*** Bug 174508 has been marked as a duplicate of this bug. ***
Comment 70 José Jeria 2002-10-15 03:14:36 PDT
*** Bug 116779 has been marked as a duplicate of this bug. ***
Comment 71 Sébastien Delahaye 2002-12-20 06:57:28 PST
*** Bug 186271 has been marked as a duplicate of this bug. ***
Comment 72 Koike Kazuhiko 2003-02-05 20:14:47 PST
A test suite by W3C:
http://www.w3.org/MarkUp/Test/HTML401/20030123/tests/sec17_7-BF-02.html
Comment 73 Pedro Lopes 2003-02-06 01:49:05 PST
See also the test
http://www.w3.org/MarkUp/Test/HTML401/20030123/tests/sec17_7-BF-01.html
related to ROWS, where the problem is clearly visible.
Comment 74 Max Kanat-Alexander 2003-04-03 11:52:53 PST
*** Bug 193296 has been marked as a duplicate of this bug. ***
Comment 75 Gérard Talbot 2003-04-11 19:58:29 PDT
Is there a particular or special reason why the keyword html4 should not be
added to this bugfile?
Comment 76 dsirnapalli 2003-05-07 20:42:35 PDT
Need traction on this bug
Comment 77 Samir Gehani 2003-05-20 09:50:05 PDT
adt: nsbeta1-
Comment 78 Grey Hodge (jX) 2003-05-23 22:35:10 PDT
*** Bug 206946 has been marked as a duplicate of this bug. ***
Comment 79 Max Kanat-Alexander 2003-06-21 23:13:53 PDT
Can't block a duplicate. That doesn't make any sense. Removing "blocks bug 116779."

Re: comment 75 -- TEXTAREA was in HTML 3.2, so html4 wouldn't be a valid keyword.

-M
Comment 80 Mats Palmgren (:mats) 2003-08-01 11:31:00 PDT
*** Bug 214744 has been marked as a duplicate of this bug. ***
Comment 81 Lorenzo Colitti 2003-12-06 10:46:39 PST
It looks as if this hasn't been looked at for a long time.
Reassigning to get it some attention.
Comment 82 alanjstr 2003-12-26 12:12:27 PST
*** Bug 215099 has been marked as a duplicate of this bug. ***
Comment 83 Josh Birnbaum 2004-02-13 07:27:15 PST
*** Bug 234163 has been marked as a duplicate of this bug. ***
Comment 84 Janick Bernet 2004-03-15 05:56:58 PST
*** Bug 237523 has been marked as a duplicate of this bug. ***
Comment 85 Janick Bernet 2004-03-15 06:11:43 PST
Created attachment 143957 [details]
Rendering of <textarea rows=1> vs <input type=text > (ie vs moz)

Rendering of textarea rows=1 and text type=input in ie and mozilla.
Comment 86 Boris Zbarsky [:bz] (still a bit busy) 2004-04-04 19:39:57 PDT
Comment 46, way back when, is right on the money.  The textarea size is the
specified cols/rows plus space for scrollbars.  Since, unlike IE, we don't show
the scrollbars unless they are needed people feel that this is broken.

Possible options:

1)  Don't add in the scrollbar sizes when computing the size of the textarea.
    That will make things be "correct" as long as there is no overflow, and
   "incorrect" otherwise.
2)  Always show the scrollbars, like IE.
3)  Include the scrollbars only when we plan to show them (this is somewhat
    circular, since the size then depends on whether we show the scrollbars,
    which depends on the size).

Thoughts?  Any other options I'm missing?
Comment 87 Robert O'Callahan (:roc) (email my personal email if necessary) 2004-04-04 20:26:58 PDT
Option 4) Let ROWS= and COLS= control the size of the textarea content only, so
when a scrollbar becomes visible, the textarea grows (assuming it's not
constrained by other layout constraints).

I don't particularly like any of these options but maybe option 4) isn't so bad...
Comment 88 Boris Zbarsky [:bz] (still a bit busy) 2004-04-04 20:29:23 PDT
That was basically my option #3.

The relevant code is in nsTextControlFrame::ReflowStandard, called from
nsTextControlFrame::GetPrefSize (mm.... box layout....)
Comment 89 Robert O'Callahan (:roc) (email my personal email if necessary) 2004-04-04 21:16:01 PDT
Oh. My option 4 is not circular though.
Comment 90 Boris Zbarsky [:bz] (still a bit busy) 2004-04-04 21:42:41 PDT
Hmm... maybe with a lot of refactoring of nsTextControlFrame reflow... (would
need to reflow at the scrollbar-less width/height to decide whether we need the
scrollbars, and then if we do reflow again at the new width/height or something?)
Comment 91 Robert O'Callahan (:roc) (email my personal email if necessary) 2004-04-04 22:43:51 PDT
I just said it would be well-defined, I didn't say it would be easy to implement
:-(.
Comment 92 Lorenzo Colitti 2004-04-05 02:21:20 PDT
(In reply to comment #86)
> Comment 46, way back when, is right on the money.  The textarea size is the
> specified cols/rows plus space for scrollbars.  Since, unlike IE, we don't
> show the scrollbars unless they are needed people feel that this is broken.

Are you sure that's the only thing going on? I see strange non-linear behaviour.
For example, look at attachment 13337 [details]. Notice that the textarea does not have
space for 30 characters (on my system, it's more like 26). That can't be due to
the scrollbars.

Now decrease the text size with CTRL-. On my system, the first time the text
gets a little smaller but the textarea's *gets wider* (and shows about 33
columns). If I decrease the font size again, it becomes narrower again (about 28
columns). That's weird. So I think something else is wrong too.

I think it's due to the textarea specifying a non-default font.
Comment 93 Boris Zbarsky [:bz] (still a bit busy) 2004-04-05 07:36:24 PDT
> Are you sure that's the only thing going on?

Yes.

> Notice that the textarea does not have space for 30 characters

Arial is a variable width font.  "30 characters" is meaningless in that case. 
We size for 30 "average characters" where the font itself reports the average
char width.  You may as well complain that you can't fit 30 'W' chars but can
fit far too many 'l' chars.

> but the textarea's *gets wider* (and shows about 33 columns)

This part I'm not seeing.  It may be due to the specifics of the font metrics of
the Arial fonts installed on your system  (or possibly even to us being unable
to find Arial at the new size and using a different font).

So the point is that the behavior for fixed-width fonts is due to the scrollbars
and probably needs fixing.  The behaviot for variable-width fonts is due to the
fact that there is no good solution there; we've gone with the solution that
most closely resembles IE's in the most cases as far as we can tell.
Comment 94 rmjb 2004-04-05 08:16:30 PDT
As to the intended behaviour, I design web forms and some of them need signed
approval. To achieve this I print the form after it's submitted, but I maintain
the form's shape/structure on the web page. This is done firstly with an
enclosing div that measures 8.5" x 11".

If I size a textarea that fits in an 8.5" space with 0.75" margins on IE I set
the width to be 80 cols. When the page loads it fits perfectly and I can enter
80 1s and the line goes right up to the (greyed out) scroll bar. But when I load
it in Fx the page is wider and the 80 1s don't go up to the edge of the text
area, infact there's space for 2 more 1s after those 80 and then the horizontal
scroll bar comes up, in IE the text wraps to the next line, even though there is
no space anywhere in the line yet. Note the width of the text area is set at 80
and the textarea is using a fixed-width font as far as I can tell.

Also note I cant get the wrapping behaviour of IE regardless of the wrap
attribute I set on the text area.

I will upload two attachments displaying the effect in both IE and Fx.

- rmjb
Comment 95 rmjb 2004-04-05 08:19:07 PDT
Created attachment 145480 [details]
Behaiuor in IE with cols=80 and 80 1s. The 1s go right up to the edge.
Comment 96 rmjb 2004-04-05 08:21:00 PDT
Created attachment 145481 [details]
Behaviuor in Fx with cols=80 and 80 1s. There's space for 2 more 1s and the scroll bar only takes up 1 char.
Comment 97 Boris Zbarsky [:bz] (still a bit busy) 2004-05-01 12:58:20 PDT
The wrapping stuff has nothing to do with this bug; it's covered by other bugs.
Comment 98 Boris Zbarsky [:bz] (still a bit busy) 2004-05-25 17:16:31 PDT
*** Bug 244687 has been marked as a duplicate of this bug. ***
Comment 99 José Jeria 2004-09-07 10:50:54 PDT
*** Bug 258272 has been marked as a duplicate of this bug. ***
Comment 100 Mats Palmgren (:mats) 2004-09-13 14:37:55 PDT
*** Bug 255651 has been marked as a duplicate of this bug. ***
Comment 101 Bill Mason 2004-09-28 00:00:32 PDT
*** Bug 261111 has been marked as a duplicate of this bug. ***
Comment 102 Hideo Saito 2004-10-12 21:08:28 PDT
There is an improvement about the height of a textarea, because the patch of Bug
167001 increases the height of an input field and textarea field.
For example, the text of textarea does not overflow downward out of the textarea
by the TextZoom.
Comment 103 Roger Thomas 2005-02-02 21:26:11 PST
Rows not right....

<textarea name='resolution' style='width:100%' rows='3'><xsl:value-of
select="document/resolution"/></textarea>

Displays 4 rows... It should only display 3 rows.   
Comment 104 Hideo Saito 2005-02-27 07:06:25 PST
Created attachment 175726 [details] [diff] [review]
patch

I try to implement the option 3) or 4) that are mentioned on comment #86 and
comment #87, although about only ROWS.
Comment 105 Hideo Saito 2005-02-27 07:55:19 PST
Created attachment 175730 [details] [diff] [review]
patch

I modified for rows=1.
Comment 106 Hideo Saito 2005-03-03 08:20:04 PST
Created attachment 176151 [details] [diff] [review]
patch for rows and cols

If the textarea has styled pixel or percent width or height, the scrollbars
appear inside the textarea, otherwise the scrollbars appear outside the
textarea.
Comment 107 Hideo Saito 2005-03-03 08:37:35 PST
Created attachment 176155 [details]
testcase for patch
Comment 108 Hideo Saito 2005-03-03 08:43:09 PST
Created attachment 176156 [details]
testcase for patch
Comment 109 Hideo Saito 2005-03-06 08:03:52 PST
Created attachment 176469 [details] [diff] [review]
patch

I modified for the reflow of LayoutBox, because max-element-width of contents
of the textarea is not updated by set mark-dirty, if the horizontal scrollbar
is removed by deleting a character.
Comment 110 Hideo Saito 2005-03-06 08:10:15 PST
Created attachment 176470 [details]
screen shot for patch

Is this behavior acceptable?
Comment 111 Boris Zbarsky [:bz] (still a bit busy) 2005-03-07 09:35:09 PST
Comment on attachment 176469 [details] [diff] [review]
patch

I don't think I can review this patch any time in the near future -- I don't
really know the scroll frame code well.

The fact that this patch makes scrollframe do something special for text
control frames does make alarm bells go off in my head, though.
Comment 112 Phil Ringnalda (:philor) 2005-03-07 22:54:18 PST
*** Bug 285228 has been marked as a duplicate of this bug. ***
Comment 113 Hideo Saito 2005-03-08 02:31:14 PST
Comment on attachment 176469 [details] [diff] [review]
patch

bzbarsky, thanks. I will ask to review to other reviewers.
Comment 114 David Baron :dbaron: ⌚️UTC-10 2005-03-08 07:55:59 PST
Comment on attachment 176469 [details] [diff] [review]
patch

Pointing review request at roc; but note bz's comment 111.
Comment 115 Robert O'Callahan (:roc) (email my personal email if necessary) 2005-03-14 15:00:19 PST
I don't think major changes to nsGfxScrollFrame should be necessary.

Can't we do roughly what Boris suggested in
https://bugzilla.mozilla.org/show_bug.cgi?id=33654#c90
?
1) nsTextControlFrame::Reflow at intrinsic size excluding scrollbars
2) if there are no scrollbars, stop.
3) else reflow at intrinsic size plus sizes of scrollbars present.
3b) we can force the scrollframe to show the scrollbars that it was showing in
step 2, even if they're not actually needed, to ensure that the textframe
non-scrollbar area is exactly the specified ROWS and COLS. For this, we could
hack GetScrollbarStyles to ask the text control frame parent if it wants to
override the styles.
http://lxr.mozilla.org/mozilla/source/layout/generic/nsGfxScrollFrame.cpp#1119
Comment 116 Hideo Saito 2005-03-15 08:26:30 PST
Created attachment 177495 [details]
screen shot of appearance and disappearance of the scrollbars
Comment 117 Hideo Saito 2005-03-15 08:30:37 PST
> I don't think major changes to nsGfxScrollFrame should be necessary.

Does this indication refer about the following changes of the patche?
However, following changes correct appearance and disappearance of the
scrollbars, please refer to attachment 177495 [details].

+  nscoord scrollAreaHeight;
+  do {
+    scrollAreaHeight = scrollAreaRect.height;
+    PRBool updateMEW = PR_FALSE;
...
+  } while (scrollAreaHeight != scrollAreaRect.height);
Comment 118 Hideo Saito 2005-03-15 08:38:33 PST
I will add to the patch following changes.

for nsTextControlFrame::Reflow
< +  const nsStyleDisplay* disp = GetStyleDisplay();
< +  if (disp->mOverflowX == NS_STYLE_OVERFLOW_SCROLL ||
< +      aReflowState.mComputedHeight != NS_INTRINSICSIZE) {
---
> +  if (aReflowState.mComputedHeight != NS_INTRINSICSIZE) {

< +  if (disp->mOverflowY == NS_STYLE_OVERFLOW_SCROLL ||
< +      aReflowState.mComputedWidth != NS_INTRINSICSIZE) {
---
> +  if (aReflowState.mComputedWidth != NS_INTRINSICSIZE) {

and for nsGfxScrollFrameInner::Layout
< +              updateMEW = PR_TRUE;
---
> +              if (!aState.GetMaxElementWidth())
> +                updateMEW = PR_TRUE;
Comment 119 Robert O'Callahan (:roc) (email my personal email if necessary) 2005-03-17 17:38:39 PST
nsTextControlFrame could control the display of its scrollbars by modifying
GetScrollbarStyles here:
http://lxr.mozilla.org/mozilla/source/layout/generic/nsGfxScrollFrame.cpp#1119
That function can check the parent to see it it's an nsITextControlFrame and if
so, ask the text control frame whether the scrollbars should be 'auto' (for step
1 in comment #115) or 'hidden' or 'scroll' (for step 3 in comment #115). That
would be the only change required to nsGfxScrollFrame by the approach in comment
#115.
Comment 120 Hideo Saito 2005-03-20 08:52:56 PST
Created attachment 178042 [details] [diff] [review]
patch
Comment 121 Hideo Saito 2005-03-20 09:00:14 PST
Created attachment 178045 [details]
testcase for overflow property
Comment 122 Hideo Saito 2005-03-20 09:07:38 PST
Created attachment 178047 [details]
screen shot of the result applied the patch
Comment 123 Hideo Saito 2005-03-20 09:31:38 PST
roc, please refer to the posted new patch of attachment 178042 [details] [diff] [review]. Is it wrong to
call |GetStyleDisplay()| at nsTextControlFrame::Reflow, although testcase of
attachment 178045 [details] is done successfully like as attachment 178047 [details]? 
Comment 124 Robert O'Callahan (:roc) (email my personal email if necessary) 2005-03-20 21:20:06 PST
I think that part of the patch is OK. The part I don't like is all the changes
to nsGfxScrollFrame. Can you reply to comment #119?
Comment 125 Hideo Saito 2005-03-21 07:40:47 PST
Comment on attachment 178042 [details] [diff] [review]
patch

I think that the changes was added according to the flow of comment 115.

There are two variables |mIncludingHbarSpace| and |needsHbarSpace| for
horizontal scrollbar. In the case of each value is true, they force to add
space of the horizontal scrollbar. There are copies of these variables in the
nsGfxScrollFrame. The value of |needsHbarSpace| is set by adding or removing
the space of the scrollbar at |nsGfxScrollFrameInner::Layout|. The change of
the size of the control frame causes to layout by parent marked dirty as
follows.

+  if (markHbarSpace || markVbarSpace)
+    parent->MarkDirty(aState);

There are not the changes that bzbarsky noticed on comment 111.
Comment 126 Robert O'Callahan (:roc) (email my personal email if necessary) 2005-03-21 12:41:44 PST
You don't need those variables. You can control scrollbar visibility by
modifying GetScrollbarStyles.
Comment 127 :Gijs Kruitbosch 2005-04-21 13:34:56 PDT
*** Bug 291356 has been marked as a duplicate of this bug. ***
Comment 128 :Gijs Kruitbosch 2005-04-21 13:41:25 PDT
*** Bug 291242 has been marked as a duplicate of this bug. ***
Comment 129 José Jeria 2005-05-24 09:06:33 PDT
*** Bug 295352 has been marked as a duplicate of this bug. ***
Comment 130 Erik Fabert 2005-08-10 06:48:59 PDT
*** Bug 304136 has been marked as a duplicate of this bug. ***
Comment 131 djn 2005-08-10 07:46:12 PDT
(In reply to comment #130)
> *** Bug 304136 has been marked as a duplicate of this bug. ***

I'd added a comment to 304136 explaining why having a 3-row high text area
appear when rows="2" is specified is unhelpful. Since that bug has been marked
as a duplicate, I'll include that comment here too, otherwise I suspect it won't
be noticed.

> I think that the last row is for the horizontal scroll bar.
> Try entering a long text with no spaces into the textarea.

The reason why this behaviour matters is that we have a web application that
requests some short summary text from the user. Our textarea is sized with
rows="2" so that the user is given a strong visual indication of how much text
is desired (more than a line, but preferably not an essay). The problem with the
behavioural inconsistency between Firefox and the other major browsers (as
exampled by IE6 and Opera 8) is that this visual indication of the amount of
required text is different on different user agents for a given rows attribute
value, and therefore the utility of that attribute is greatly diminished.

I shall now attempt to convince that following IE/Opera would be superior to the
current Firefox behaviour :-)

The HTML 4.01 spec says of the cols attribute:

"Users should be able to enter longer lines than this, so user agents should
provide some means to scroll through the contents of the control when the
contents extend beyond the visible area. User agents may wrap visible text lines
to keep long lines visible without the need for scrolling."

Thus, although providing a scrolling mechanism is allowed, it is not required,
as forcibly wrapping the visible text lines is also an option that a user agent
can take.

1. The desire for broad consistency with other browsers (where they are
conformant with the relevant recommendations) would suggest that behaving in the
same way as IE and Opera would be preferable. As already described, behaving
inconsistently lessens the usefulness of sizing textareas to give usability
hints to the user.

2. Since Firefox will already wrap the content text if it contains spaces, the
need for a horizontal scrollbar to appear is actually likely to be extremely
rare. This is because users will generally be entering words separated by spaces
(thus giving the user agent opportunities for wrapping lines at spaces) into the
textarea. The case where a long sequence of characters unbroken by a space is
entered must surely be far less common. Therefore, sizing the textarea correctly
for the least common case seems less than optimal. The alternative of sizing the
textarea to be the correct height for the specified number of rows and then
having the horizontal scrollbar eat into that space when/if it appears, would be
just as viable. Thus, there exists a better alternative to the current
behaviour, even if it is deemed that following the behaviour of IE and Opera is
not desirable.

3. (2) might be objected to on the grounds that a textarea with attribute
rows="1" would consist purely of a horizontal scrollbar, if an unbroken string
of non-space characters were entered. However, this is an extremely unlikely
scenario -- a text input field would almost certainly be used by a page designer
in preference to a textarea one row high. In any case, if thought to be a
serious objection, the rows="1" situation could be treated as a special case.

4. Intuition (i.e. the desireability of having the user agent do the least
surprising with a given chunk of HTML) would suggest that it is unhelpful to
have a textarea *three* rows high output when a textarea *two* rows high is
requested.
Comment 132 Bart Szyszka 2005-12-07 13:34:12 PST
In addition to Comment #131, I'd just like to add that Firefox's behavior is also incorrect when the contents of a textarea don't scroll. It seems like Firefox was designed for a textarea that always gets overfilled, but what about instances where a textarea is for an address, where it is unlikely to ever have more than three rows in 99% of that form's uses? Firefox always displays an extra row, even if I have rows="3", unecessarily adding bulk to the page. It's also confusing to see a box taller than it should be, making the user think they haven't filled in enough information.

Plus it just doesn't make any sense from an HTML coding point of view to see four rows when I've specified three. If a web developer would find it undesirable to have a horizontal scrollbar decrease the viewable area of a text box, then they should add an extra row themselves, not have Firefox do it for them prematurely.
Comment 133 Hideo Saito 2005-12-16 17:17:33 PST
Created attachment 206150 [details]
testcase for patch
Comment 134 Hideo Saito 2005-12-16 17:21:05 PST
Created attachment 206151 [details] [diff] [review]
patch
Comment 135 Hideo Saito 2005-12-16 17:36:19 PST
(In reply to comment #126)
> You don't need those variables. You can control scrollbar visibility by
> modifying GetScrollbarStyles.

roc, is this patch(attachment 206151 [details] [diff] [review]) wrong yet, because the changes does not modify GetScrollbarStyles?
Comment 136 Hideo Saito 2005-12-17 09:41:09 PST
Comment on attachment 206151 [details] [diff] [review]
patch

Following boolean variables are used and conrtol scrollbar visibility. If these variables have default value as follows, the changes are of no effect. If the parent has interface to nsITextControlFrame, these variables are set according to comment 115.

mInsideVbarSpace = PR_TRUE; // for a style, space for scrollbar is inside of the frame
mInsideHbarSpace = PR_TRUE;
hasVbarSpace = PR_TRUE;  // the frame actually has a space for scrollbar
hasHbarSpace = PR_TRUE;
hasVScrollbar = PR_FALSE;  // if true, scrollbar is visible
hasHScrollbar = PR_FALSE;
Comment 137 Robert Lee 2005-12-30 10:18:24 PST
(In reply to comment #86)
Option 5: Always show the vertical scroll bar. If wrapping is not enabled always show the horizontal scroll bar. If wrapping is enabled hide the horizontal scroll bar and do not add space for it. If wrapping is enabled make it impossible for text to extend beyond the width of the text area even if a line contains no spaces.

In this way, scroll bars are not dynamically added and removed at all (like in IE) so extra space never needs to be allocated for them. In most cases the horizontal scroll bar will never appear (wrapping is enabled). And in cases where a page author disabled wrapping he should probably expect a horizontal scroll bar to be visible anyway.

The presence of the vertical scroll bar will also have the affect of indicating to the end user that the box on thier screen is a container which expects text. As it is rendered now, an empty textarea on WinXP provides almost no cue to the user as to what the box is until the user moves the mouse over it and gets the I-beam cursor (WinXP renders the border as a 2D box).

Scroll bars should be disabled when no text extends beyond the bounds of the textarea as expected.
Comment 138 mbaliel 2006-03-04 14:16:34 PST
In reply to comment #131/132.......

I would like to see this feature work appropriately in FF.  We use 1 row text areas as inputs because they respond to width=[Percentage%].
Comment 139 Mark Romero 2006-04-01 11:47:27 PST
*** Bug 332451 has been marked as a duplicate of this bug. ***
Comment 140 Vitaly Harisov 2006-07-04 12:18:24 PDT
*** Bug 343576 has been marked as a duplicate of this bug. ***
Comment 141 Vitaly Harisov 2006-07-04 12:24:11 PDT
Yet another extra line example:
http://vitaly.harisov.name/bugs/one-row-textarea.html
Comment 142 Gérard Talbot 2007-01-21 16:01:07 PST
*** Bug 255038 has been marked as a duplicate of this bug. ***
Comment 143 Steve Brecht 2008-06-01 22:48:28 PDT
This one continues to be an issue for me as well when trying to create accurate cross browser layouts.
Comment 144 Hideo Saito 2008-08-21 07:42:21 PDT
Created attachment 334890 [details] [diff] [review]
patch

I tried to modify previous patch using |GetScrollbarStyles| and |SetScrollbarStyles| in order to control styles of the scrollbars. New variables are used as follows,

    mInside[VH]barSpace -- if true, the space is inside a textarea
    mNeeds[VH]barSpace -- if true, the space will be appended by |SetScrollbarStyles|
    mHas[VH]barSpace -- if true, the space was appended

The changes to |nsHTMLScrollFrame::TryLayout| are from the reason that the appended spaces are not used to place the text. When the scrollbars appear or disappear, I use |FrameNeedsReflow| to start the reflow at changes of |nsHTMLScrollFrame::Reflow|. However, in the midst of a reflowing, it seems that |FrameNeedsReflow| does not work, so that I check the condition and start it after in |ProcessReflowCommands|.
Comment 145 Hideo Saito 2008-08-23 17:26:10 PDT
Created attachment 335214 [details] [diff] [review]
patch

I added a new interface |NeedsReflow| to |nsITextControlFrame|, it is called when scrollbars are appearing or disappearing and it clears |mPrefSize| just now.
Comment 146 Masayuki Nakano [:masayuki] (Mozilla Japan) 2008-08-23 23:27:14 PDT
Saito-san, please request review and sr if the patch is completed.
Comment 147 Hideo Saito 2008-08-24 07:30:26 PDT
Comment on attachment 335214 [details] [diff] [review]
patch

roc, could you review this patch?

I intend to influence behavior only when there is a scrollbar outside the textarea. The decision as to whether to include the space of the scrollbar is in |SetScrollbarStyles| and |GetScrollbarStylesFromFrame|.
Comment 148 Boris Zbarsky [:bz] (still a bit busy) 2008-08-24 17:25:20 PDT
You shouldn't need the SizeNeedsRecalc call, since FrameNeedsReflow handles that, no?
Comment 149 Hideo Saito 2008-08-24 20:40:20 PDT
(In reply to comment #148)
> You shouldn't need the SizeNeedsRecalc call, since FrameNeedsReflow handles
> that, no?
> 

Yes, I think so. However, FrameNeedsReflow is called after at PresShell::ProcessReflowCommands. As the result, the following message is displayed from nsTextControlFrame::ComputeAutoSize. I think that mPrefSize should be cleared immediately. If it is not necessary, new interface NeedsReflow is also not needed.

    NS_ASSERTION(ancestorAutoSize.width == autoSize.width,
                 "Incorrect size computed by ComputeAutoSize?");
Comment 150 Boris Zbarsky [:bz] (still a bit busy) 2008-08-25 08:33:48 PDT
> However, FrameNeedsReflow is called after at PresShell::ProcessReflowCommands

Er... no.  It's not.  The relevant part of your code is:

+  presShell->FrameNeedsReflow(this, nsIPresShell::eTreeChange, NS_FRAME_IS_DIRTY, PR_TRUE);
+  SizeNeedsRecalc(mPrefSize);

That first call does a SizeNeedsRecalc, right?  Why is the second call needed?
Comment 151 Hideo Saito 2008-08-25 15:17:05 PDT
> That first call does a SizeNeedsRecalc, right?  Why is the second call needed?

The first call does not do the SizeNeedsRecalc, because I changed the code of FrameNeedsReflow as follows.

+  if (aForce && mIsReflowing) {
+    struct NeedsReflowParam* param = new (struct NeedsReflowParam);
+    if (param) {
...
+      return NS_OK;
+    }
+  }

If there is not the second call, many warning "Incorrect size computed by ComputeAutoSize?" is asserted.
Comment 152 Boris Zbarsky [:bz] (still a bit busy) 2008-08-25 17:55:22 PDT
Ah.  I think that change to FrameNeedsReflow is probably wrong, offhand.  Shouldn't you just add a post-reflow callback to trigger the reflow instead of doing that?
Comment 153 Hideo Saito 2008-08-25 18:31:42 PDT
(In reply to comment #152)
> Ah.  I think that change to FrameNeedsReflow is probably wrong, offhand. 
> Shouldn't you just add a post-reflow callback to trigger the reflow instead of
> doing that?
> 

If there is not the change to FrameNeedsReflow, the warning "can't mark frame dirty during reflow" is asserted, certainly, the reflow does not work. Should I search other approach without that change?
Comment 154 Boris Zbarsky [:bz] (still a bit busy) 2008-08-25 19:04:28 PDT
I guess my question is why you're hacking "do stuff after the reflow finishes" functionality into FrameNeedsReflow and calling that during reflow, when we already have a similar setup with nsIReflowCallback.  Is nsIReflowCallback not sufficient for what you need out of it?  (Note: I haven't read most of the patch, just the textcontrol frame part.)
Comment 155 Hideo Saito 2008-08-26 05:20:22 PDT
Boris, thanks for your advice, my approach was quiet insufficient. I think that I can suppress the extra changes to nsTextControlFrame and nsPresShell.
Comment 156 Hideo Saito 2008-08-26 05:25:06 PDT
Created attachment 335521 [details] [diff] [review]
patch

I use MarkIntrinsicWidthsDirty instead of SizeNeedsRecalc(mPrefSize) and move the FrameNeedsReflow to ReflowFinished.
Comment 157 Erik Fabert 2009-04-22 11:28:38 PDT
*** Bug 489598 has been marked as a duplicate of this bug. ***
Comment 158 Ben Wallis 2009-05-08 08:37:11 PDT
Textarea tag displays one more row than specified in "rows" attribute.

I specify rows="4", Firefox shows 5 rows. Other browsers show 4 rows (Safari 4, Chrome, IE8, Opera 9).
Comment 159 Robert O'Callahan (:roc) (email my personal email if necessary) 2010-03-01 17:00:41 PST
Comment on attachment 335521 [details] [diff] [review]
patch

Needs to be updated to trunk. Sorry that I forgot about this review.
Comment 160 Hideo Saito 2010-04-05 21:11:25 PDT
Created attachment 437216 [details] [diff] [review]
patch

I updated the patch on the trunk.

roc, could you review this patch?
Comment 161 Robert O'Callahan (:roc) (email my personal email if necessary) 2010-04-05 21:28:06 PDT
+  PRPackedBool mInsideVbarSpace:1;
+  PRPackedBool mInsideHbarSpace:1;
+  PRPackedBool mNeedsVbarSpace:1;
+  PRPackedBool mNeedsHbarSpace:1;
+  PRPackedBool mHasVbarSpace:1;
+  PRPackedBool mHasHbarSpace:1;

I'd like to avoid the need for these variables. I'd also like to avoid the need for SetScrollbarStyles().

Maybe we can do all of this from nsTextControlFrame? Call GetActualScrollbarSizes before and after we reflow the scrollframe child. If a scrollbar appeared or disappeared and we're using ROWS/COLS, issue a post-reflow callback to reflow ourselves. Keep the current change to nsTextControlFrame::CalcIntrinsicSize but don't call SetScrollbarStyles.
Comment 162 Hideo Saito 2010-04-16 03:24:01 PDT
Created attachment 439497 [details]
testcase for patch on wrap="off"
Comment 163 Hideo Saito 2010-04-16 03:38:12 PDT
Created attachment 439499 [details] [diff] [review]
patch

> I'd like to avoid the need for these variables. I'd also like to avoid the need
> for SetScrollbarStyles().

I try to do so on your advice, but following variables still remained.

+  PRPackedBool mInsideHbarSpace:1;
+  PRPackedBool mInsideVbarSpace:1;
+  PRPackedBool mHasHbarSpace:1;
+  PRPackedBool mHasVbarSpace:1;

These variables needs to control the appearance of the scrollbars outward or the disappearance. I think that this change doesn't influence in case of the space in the inside of the textarea.

I saw another bug during a test, but I want to fix here because it is related to this change closely.

In case of the space in the inside of the textarea and the attribute for the wrap text isn't off, the vertical scrollbar that appeared once doesn't disappear even if the several typed letters are deleted. Following change is related to this bug. The function |Contains| isn't true because it is assumed that there is vertical scrollbar in measuring width of scrolledRect.

-    nsSize insideBorderSize =
-      ComputeInsideBorderSize(aState,
-                              nsSize(kidDesiredSize.width, kidDesiredSize.height));
-    nsRect scrolledRect =
-      mInner.GetScrolledRectInternal(kidDesiredSize.mOverflowArea, insideBorderSize);
-    if (nsRect(nsPoint(0, 0), insideBorderSize).Contains(scrolledRect)) {
+    PRBool force = mInner.mInsideVbarSpace && aState->mReflowedContentsWithVScrollbar;
+    if (!force) {
+      nsSize insideBorderSize =
+        ComputeInsideBorderSize(aState,
+                                nsSize(kidDesiredSize.width, kidDesiredSize.height));
+      nsRect scrolledRect =
+        mInner.GetScrolledRectInternal(kidDesiredSize.mOverflowArea, insideBorderSize);
+      force = nsRect(nsPoint(0, 0), insideBorderSize).Contains(scrolledRect);
+    }
+    if (force) {
Comment 164 Robert O'Callahan (:roc) (email my personal email if necessary) 2010-04-16 04:00:45 PDT
(In reply to comment #163)
> I try to do so on your advice, but following variables still remained.
> 
> +  PRPackedBool mInsideHbarSpace:1;
> +  PRPackedBool mInsideVbarSpace:1;
> +  PRPackedBool mHasHbarSpace:1;
> +  PRPackedBool mHasVbarSpace:1;

Can you explain in a comment exactly what these variables mean?

> I saw another bug during a test, but I want to fix here because it is related
> to this change closely.

Can you put it in a separate patch? That will make this easier to review.
Comment 165 Hideo Saito 2010-04-17 07:42:04 PDT
Created attachment 439698 [details] [diff] [review]
base patch

I separated the patch and modified the comment.
Comment 166 Hideo Saito 2010-04-17 07:52:56 PDT
Created attachment 439699 [details] [diff] [review]
additional patch for another bug

Could you also review this additional patch?
Comment 167 Robert O'Callahan (:roc) (email my personal email if necessary) 2010-04-18 17:04:25 PDT
I still don't really understand what you mean by "inside" and "outside" here. Can you explain it, perhaps with a screenshot?
Comment 168 Hideo Saito 2010-04-18 17:59:58 PDT
Created attachment 439825 [details] [diff] [review]
base patch

I want to modify nsTextControlFrame.cpp and nsTextControlFrame.h to remove following variables.

+  // controls space of the scrollbars
+  PRPackedBool mtcshHasHbarSpace;
+  PRPackedBool mHasVbarSpace;

> I still don't really understand what you mean by "inside" and "outside" here.
> Can you explain it, perhaps with a screenshot?

The "inside" is meaning that the scrollbar appear inward as follows.
The scrollable frame's size does not change.

    +-----------+
    + 1         +
    + 2         +
    +_3_________+

    +--------+--+
    + 2      |^ +
    + 3      |  +
    +_4______|__+

The "outside" is meaning that the scrollbar appear outward as follows.
The scrollable frame's size does change.

    +---------+
    + 1       +
    + 2       +
    +_3_______+

    +---------+--+
    + 2       +^ |
    + 3       +  |
    +_4_______+_ |
Comment 169 Robert O'Callahan (:roc) (email my personal email if necessary) 2010-04-22 04:41:22 PDT
OK.

I really don't think we need to add new nsGfxScrollFrameInner state here. We should be able to do everything from nsTextControlFrame.

You can add an API to nsIScrollableFrame to disable scrollbars directly. It would modify mNeverHasVerticalScrollbar/mNeverHasHorizontalScrollbar.

Then when 'rows' is specified, nsTextControlFrame will directly enable/disable the horizontal scrollbar and when 'cols' is specified, it will enable/disable the vertical scrollbar. Make nsTextControlFrame::ComputeIntrinsicSize not include space for scrollbars you have explicitly disabled.

In nsTextControlFrame::Reflow, after reflowing the scrollframe check if you need to try to enable or disable each scrollbar that you are directly controlling. If content is overflowing in a direction where the scrollbar is disabled, you should enable it. If a scrollbar is not being shown in a given direction, you should disable it.

Whenever you enable/disable a scrollbar in this way, you'll need to mark yourself as needing reflow in a post-reflow callback.

Can you try that?
Comment 170 Hideo Saito 2010-05-01 09:15:02 PDT
Created attachment 442925 [details] [diff] [review]
patch

How about this patch, although I have to do enough test?

I use mNeverHasVerticalScrollbar/mNeverHasHorizontalScrollbar to disable the scrollbars via a new API |SetScrollbarSpace|.

New boolean mForceHasHorizontalScrollbar and mForceHasVerticalScrollbar are used to enable the scrollbars via the |SetScrollbarSpace|.

New boolean mCannotHasHorizontalScrollbar and mCannotHasVerticalScrollbar are used to ask to disable the scrollbars by reason that it does not have enough space. In this case, the appended space for scrollbars is removed, and these boolean keep the state not to repeat enable and disable.

I get the overflow state via a new API |GetScrollbarOverflowState| instead of |GetActualScrollbarSizes|.

In |TryLayout|, if the scrollbars can not be enabled, I stop the reflow and set boolean mCannotHasHorizontalScrollbar/mCannotHasVerticalScrollbar and mNeedsReflowParentFrame to restart the reflow.
Comment 171 Robert O'Callahan (:roc) (email my personal email if necessary) 2010-05-01 13:52:56 PDT
Can you explain why comment #169 didn't work?
Comment 172 Hideo Saito 2010-05-03 06:02:37 PDT
> You can add an API to nsIScrollableFrame to disable scrollbars directly. It
> would modify mNeverHasVerticalScrollbar/mNeverHasHorizontalScrollbar.

I used mNeverHasVerticalScrollbar/mNeverHasHorizontalScrollbar to disable scrollbars. However, these boolean can't be used to enable scrollbars because the appended space is used to put content. I add new boolean mForceHasHorizontalScrollbar/mForceHasVerticalScrollbar so that these boolean force to display the scrollbars on the space.
Comment 173 Hideo Saito 2010-05-06 18:10:14 PDT
Created attachment 444005 [details] [diff] [review]
patch which does not remove space

This does not use mCannotHasHorizontalScrollbar/mCannotHasVerticalScrollbar because the scrollbar is displayed even if the space is too narrow.
Comment 174 Hideo Saito 2010-05-16 18:28:29 PDT
Created attachment 445636 [details] [diff] [review]
patch which does not remove space

I added a change likewise to behave as to appearance/disappearance of the scrollbars even if the frame is resized on the corner.
Comment 175 Alice0775 White 2011-02-12 04:25:25 PST
*** Bug 633717 has been marked as a duplicate of this bug. ***
Comment 176 Mounir Lamouri (:mounir) 2011-02-13 10:15:12 PST
*** Bug 633717 has been marked as a duplicate of this bug. ***
Comment 177 Mounir Lamouri (:mounir) 2011-02-13 10:18:43 PST
Comment on attachment 442925 [details] [diff] [review]
patch

Asking review to Roc to have feedback on these patches.
Comment 178 Mike Moening 2012-11-30 10:59:00 PST
Looks like we have a patch.  Can this patch be reviewed?
It would be nice to see movement on this...
Comment 179 Drew Willcoxon :adw 2013-08-30 12:15:11 PDT
*** Bug 911226 has been marked as a duplicate of this bug. ***
Comment 180 santosh 2014-04-22 15:43:18 PDT
Hi Guys, 
No need o change any browser based code, since just by adding following style in css resolve this issue

textarea{
   overflow-x: hidden
}

Note: it is only applicable when there is no horizontal scroll bar required
Comment 181 santosh 2014-04-22 15:43:59 PDT
Hi Guys, 
No need o change any browser based code, since just by adding following style in css resolve this issue

textarea{
   overflow-x: hidden
}

Note: it is only applicable when there is no horizontal scroll bar required
Comment 182 aprjanka 2014-04-30 00:35:53 PDT
@santosh: This bug cannot be fixed with css owerflow.

Firefox still the only browser in the earth which renders a tetxarea adding one extra row.
Now we got a nobody asked for chrome ui redesing in ver 29, instead of fixing a 14 years old primitive bug...
Comment 183 manohoo 2016-07-20 08:27:44 PDT
16 years since this bug was reported and hasn't been fixed?
Comment 184 Nox 2016-08-30 00:58:18 PDT
wow... soon 2017 and still not fixed... css isn't the solution

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