Closed Bug 16813 Opened 25 years ago Closed 25 years ago

[DOGFOOD][4.xP] Textarea filled from bookmark has wrong line breaks

Categories

(Core :: Layout: Form Controls, defect, P1)

Product:
Core
Component:
Layout: Form Controls
Platform:
x86
Linux
Type:
defect

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: dbaron, Assigned: sfraser_bugs)

References

Details

(Whiteboard: [PDT+] [by 12/10])

Attachments

(2 files)

test case with four variants
407 bytes, text/html
Details
same testcase, 4.x compatible (inside form element)
468 bytes, text/html
Details 
DESCRIPTION:
  There are at least four textarea regressions from two days ago (although the
CR and LF entities bug is fixed).  They are the following:


 * Motion keys do not work.  Up, down, home, end pgup, pagedn, right, left.
 * Any hard returns are double-submitted (at least when wrap=hard).  That is, I
pressed return on the line between these bullet points, but they're going to
come out with a blank line between them, even though I didn't write that.

 * I get horizontal scrollbars when I don't need them.
 * The only way to delete a newline character is to put the cursore *after* the
first character on the next line and hit backspace, instead of before it.


STEPS TO REPRODUCE:


 * Fill out a bugzilla form like I'm doing now.


DOES NOT WORK CORRECTLY ON:
 * Linux, apprunner, 1999-10-19-08-M11


Sorry I don't have time to write this bug up more formally...
Motion key problems are covered in bug 16795 I believe.
Actually, the returns problem seems to involve only turning single returns to
double.
That's not it either, the extra return comes after any hard return on a line
that actually had to be wrapped to multiple lines...
Assignee: karnaze → buster
Reassigning to Steve.
I actually suspect the extra newlines thing has to do with editing around the
default value of a textarea - and has therefore only shown up since bug 16624
was fixed.

If you look at bug 16848, which I filed using 10-19-08-M11 Linux apprunner, it
says:

DESCRIPTION:
 In my test page for Document interfaces I'm getting a crash
with the following stack trace:

where I typed:

DESCRIPTION: In my test page for Document interfaces I'm getting a crash
with the following stack trace:

The "DESCRIPTION:" was in my Bugzilla template, along with entities as described
in bug 16624.
OK... this bug should probably be split.  If I number the original regressions,
(1) is already bug 16795, (2) and (4) are both related to entities in default
values, I think, and (3) is a separate bug relating to scrolling (and there's
actually more to add to that - that carrying occurs too late, widthwise, which
is probably what's causing the horizontal scrollbar to be needed.)


Do you want me to split it?
yes, it would be very helpful to have this bug split out into separate problem
reports, since different people will work on different problems.  thanks!
Status: NEW → ASSIGNED
Depends on: 16624
Summary: textarea regressions → problems with CR and LF entities in textareas
Spun off (3) into bug 16936.

Now this bug only covers (2) and (4).  My original analysis of these was
completely wrong.  Retitling appropriately.  There are some other tests I'd like
to write for this bug, but I really have other things to do (homework!).  I
suspect you're displaying CR as a space or something like that..., but going
forward and back or submitting the form turns it into a proper return...
Here's a more formal series of steps to reproduce this bug:
 * load http://bugzilla.mozilla.org/enter_bug.cgi
 * select "Browser"
 * Go to the description field and type the following text (between lines of
dashes):
------------------------------------
DESCRIPTION:

STEPS TO REPRODUCE:

WORKS INCORRECTLY ON:
 * Linux apprunner 1999-10-

ADDITIONAL INFORMATION:
------------------------------------
 * Hit the button "Remember values as a bookmarkable template"
 * click "to this link"
 * carefully examine the resulting textarea.

Plenty of things are wrong with this text area.  For example:

* put the cursor on the word "DESCRIPTION" and hit the down arrow.  This moves
you down 2 lines instead of one
* click the mouse after the end of the line "STEPS TO REPRODUCE" (so the cursor
is after the colon), hit enter, cilck the same place again (so the cursor is
again after the colon), and type a letter.  The letter appears at the beginning
of the next line instead of after the colon
* actually submit the form.  It gets submitted with extra blank lines in a bunch
of places.
Target Milestone: M14
setting to M14
M14?  This blocks using bookmarked templates to file bugs in Bugzilla, which is
something QA wants, I think.
The above testcase shows the following:
 * The problem of moving down two lines on a downarrow happens whenever there is
an empty line that was created from the default value, whether that default
value involves entities or not.
 * similarly editing problems only depend on empty lines from default values.
In the testcase, you can see editing problems by placing the cursor after the
"A" and hitting backspace.  The cursor ends up on the previous line.
 * Initially, CR entities (
) show as spaces, while LF entities and
characters show as linebreaks.  I might try playing with CR-LF (DOS) and CR
(Mac) files later.
Note that 4.x treats all four tests correctly.

Furthermore, note that the last test case behaves the same way in mozilla if I
change it to CR-LF (DOS format).
Summary: problems with CR and LF entities in textareas → [4.xP]problems with CR and LF entities in textareas
Summary: [4.xP]problems with CR and LF entities in textareas → [DOGFOOD][4.xP]problems with CR and LF entities in textareas
Marking DOGFOOD because I think it is.  If I shouldn't be doing that, yell.
Whiteboard: [PDT+]
Putting on PDT+ radar.
Blocks: 12658
Target Milestone: M14 → M12
now that it is PDT+, moving it over to M12 and linking to tracking bug 12658
Whiteboard: [PDT+] → [PDT+] [by 12/10]
this has not been investigate, but buster suspects that we're putting illegal
CR-LF's into the content model, when we should only be adding CR (\n), he may
need help from troy or nisheeth
Severity: normal → major
Priority: P3 → P1
Assignee: buster → sfraser
Status: ASSIGNED → NEW
I'll take a look
Status: NEW → ASSIGNED
Summary: [DOGFOOD][4.xP]problems with CR and LF entities in textareas → [DOGFOOD][4.xP] Textarea filled from bookmark has wrong line breaks
The bookmark gets stored as:

    <DT><A HREF="http://bugzilla.mozilla.org/enter_bug.cgi?bug_status=NEW&
reporter=sfraser%40netscape.com&product=Browser&version=other&priority=P3&
rep_platform=Other&op_sys=other&bug_severity=normal&maketemplate=Remember+values+
as+bookmarkable+template&assigned_to=&cc=&bug_file_loc=&short_desc=&comment=
DESCRIPTION%0D%0DSTEPS+TO+REPRODUCE%0D%0DWORKS+INCORRECTLY+ON%0D+Mac+
OS%0D%0DADDITIONAL+INFORMATION%0D&form_name=enter_bug" ADD_DATE="943328667">Enter
Bug</A>

Notice that Returns are being stored as %0D (CR). CC:ing rjc, Mr Bookmarks,
adjusting summary.
Interestingly, the URL created by 4.5 (Mac) contains encoded CRLF line breaks, as
does Mozilla (Windows). So we may want to standardize on those.

However, we should still be filtering line breaks when putting data into the
content model.
I think we need to normalize line breaks in
nsHTMLTextAreaElement::SetDefaultValue(), but can find no convenient routine to
do this. Nor does nsString seem to have a string replace function that I could
use.
Patch:

Index: nsHTMLTextAreaElement.cpp
===================================================================
RCS file: /cvsroot/mozilla/layout/html/content/src/nsHTMLTextAreaElement.cpp,v
retrieving revision 1.34
diff -r1.34 nsHTMLTextAreaElement.cpp
376c376
<   // trim leading whitespace
---
>   // trim leading whitespace. -- why?
378,380c378,391
<   nsString value(aDefaultValue);
<   value.Trim(whitespace, PR_TRUE, PR_FALSE);
<   mInner.SetAttribute(kNameSpaceID_HTML, nsHTMLAtoms::value, value, PR_TRUE);
---
>   nsString defaultValue(aDefaultValue);
>   defaultValue.Trim(whitespace, PR_TRUE, PR_FALSE);
>
>   // normalize line breaks. Need this e.g. when the value is coming from a URL,
>   // which used platform line breaks.
>   // first convert CRLF pairs to LF
>   PRInt32  offset;
>   while ((offset = defaultValue.Find("\r\n")) != kNotFound)
>   	defaultValue.Cut(offset, 1);		// remove the CR
>
>   // now replace remaining CRs with LFs
>   defaultValue.ReplaceChar("\r", '\n');
>
>   mInner.SetAttribute(kNameSpaceID_HTML, nsHTMLAtoms::value, defaultValue,
PR_TRUE);
This problem really has nothing to do with bookmarks, since it can be reproduced
by just clicking on the "bookmarkable" link.

However, I think you've uncovered a form submission bug on Linux.  I think 4.x
sent CR-LF (%0D%0A) for line-breaks in textareas, but seamonkey is sending just
LF (%0A).  (I assume you meant %0A when you said %0D.)  This might be a recent
change, since it prevents me from seeing this bug on a template bookmarked using
Mozilla (as opposed to one bookmarked in 4.x and copied to my Mozilla
bookmarks).

However, that patch definitely looks like a good thing... but does it fix all
the problems I mentioned?
(Actually, you did mean %0D.  You must have bookmarked that on Mac, which means
this form submission bug probably exists on Mac too...  or is there not a need
to submit CR-LF for compatibility.   What do other browsers do?)
I was addressing the issue of filling in the default value of textareas using the
correct line termination (since all content in gecko is LF terminated). You are
correct that we also need to ensure that we submit forms with the correct line
breaks.

karnaze: what line termination should be used when textareas are submitted?
Should this differ by platform, and where does the translation (if any) need to
happen?
CCing Eric and Steve.
I spun off the form submission bug into bug 19954.
BTW, HTML 4.0 answers sfraser's question - it says newlines should always be
CR-LF.  That seems to be what NN 4.x does (on Linux, anyway, which is an LF
platform).  But that should go on bug 19954... this bug has enough on it
already.
Correcting result of midair collision, even though I don't know if it's needed
anymore.

I could've sworn the midair collision detection said the only change karnaze
made was to the comments...
This patch looks golden to me.  Go ahead and check it in.  I've checked and Nav
on Linux submits in this way, so we're actually helping backwards compatability
here too.  :)

Feel free to mark me as the reviewer of this change.
pollmann - your last comment seems to fit better on bug 19954 than this one.
Could you have written it on the wrong bug?
Waiting for resolution on this and 19954 which are related; waiting for parser

people to return from vacation. Will address on 11/29
Yes, in my above comment, the bit about this being backwards compatable with Nav
applies to bug 19954.

I do, however, think that both changes are a good thing.  Specifically, the
patch above and some yet-to-be-determined patch in nsFormFrame to fix bug
19954.  We need to have platform neutral linefeed bahaviour, and both of these
fixes are needed to achieve that.  Simon, thanks for chasing this one down.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Checked in the patches here and in bug 19954, both of which address the problem.
rickg says that the conversions in the parser will at some stage go away.
To verify, follow dbaron's 10/28/99 20:39 steps.

Note that you should verify this bug on all 3 platforms, since line termination
issues can be platforms specific.
QA Contact update.
Dave (dbaron), if I did the testcase correctly (the 11/05/99 14:37
attachment/test case) this appears to still be happening with the 1999120308
Linux6 mozilla build.  Could you verify this on your machine also?

Using your testcase on Win98, I'm seeing a box char appearing for &#013 in tests
1) & 3).  Additionally, while on Linux test 3) looks like this:
--------------------------
|Line
|
|Another line.
|
|
--------------------------

on Win98 it looks like this:
--------------------------
|Line|_||_|Another line.
|
|
|
|
--------------------------

where '|_|' equals the box char.

Will test on MacOS as soon as I resurect my machine...in the meantime Dave, if
you wouldn't mind giving this a quick look-see, I'd really appreciate it.
Thanks!
Status: RESOLVED → REOPENED
Why isn't the bug reopened? Doing so, based on those last comments.
Resolution: FIXED → ---
Uhhh...cuz I'm a coffee-zombie?  Sorryaboutthat...clearing FIXED resolution...
ckritzer@netscape.com: please describe your steps to test, in your own words,
because the things I try seem work fine.
Linux looks fine to me, still waiting for my Window's build...
Blocks: 20870
Works on Linux 1999120508 mozilla build...checking other platforms...
Status: REOPENED → RESOLVED
Closed: 25 years ago → 25 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
Okay, this works on WinNT 1999120516 mozilla and MacOS86 1999120508 netscape as
well.

Changing resolution to fix and marking VERIFIED FIXED.
No longer blocks: 20870
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: