103833 - [FIX]no newline with preformatted text when there is quotation mark at the end of the line

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect, P2)

Description

[Thomas Schulz]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:0.9.4) Gecko/20010913
BuildID:    2001091311

In the page at http://www.newsaktuell.be/en/story.htx?nr=200001868 there is a
preformatted text with a fixed line length (69 characters).
Mozilla takes the line following an line with ending on a quotation mark and put
it on the same line (see middle of the page).
The problem can also reproduced with Netscape 6.1 on Linux and WinNT.

Reproducible: Always
Steps to Reproduce:
1.load URL http://www.newsaktuell.be/en/story.htx?nr=200001868
2.
3.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Looks like " eats the following ^M

Over to parser.

Comment 2

[harishd]

Attachment: Testcase

Comment 3

[harishd]

--> m1.0

Comment 4

[harishd]

mass move to 1.1

Comment 5

[Heikki Toivonen (remove -bugzilla when emailing directly)]

[was 1.1alpha]

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 146386 [details] [diff] [review]
Fix

So the deal here is that ReadWhitespace() will just leave a lone \r as is, and
the caller (CWhitespaceToken::Consume()) will strip out \r's.

I decided to do this instead of appending on every \n or \r (which would allow
caller to not strip), because I suspect this is faster for pages using \r\n or
\n as newline separator....

Comment 8

[Peter Van der Beken [:peterv]]

Comment on attachment 146386 [details] [diff] [review]
Fix

> Index: htmlparser/src/nsScanner.cpp
> ===================================================================

>    while(!done && current != end) {
>      switch(theChar) {
>        case '\n':
>        case '\r': ++aNewlinesSkipped;
> -      case ' ' :
> -      case '\b':
> -      case '\t':
>          {

Maybe move |++aNewlinesSkipped;| inside the brace?

I don't know much about this code, but couldn't you just replace that lone CR
with an LF?

It's a bit of a shame that the two ReadWhitespace functions now have different
behaviour.

Comment 9

[Boris Zbarsky [:bzbarsky]]

> Maybe move |++aNewlinesSkipped;| inside the brace?

Will do.

> couldn't you just replace that lone CR with an LF?

Yes, but the iterator math needed to make sure this is done right didn't seem
worth the trouble...

> It's a bit of a shame that the two ReadWhitespace functions now have different
> behaviour.

The other function just sets iterators, so the only way to change its behavior
would be to actually modify the underying scanner buffer.... that seemed scary.
Frankly, we should write a separate view source tokenizer (been talked about
much) and remove that other function from here.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Checked in.

Comment 11

[Boris Zbarsky [:bzbarsky]]

*** Bug 150981 has been marked as a duplicate of this bug. ***