Bug 3370 – Page renders in the top portion of the page...

• Status: VERIFIED
• Resolution: FIXED
• Component: Layout and Rendering

[Brian Bartlett]

The login page, and all subsequent pages render only in the top portion of the browser window.  The is a 
scroll bar for that portion, and the entire page is properly render within the scrollable section.

This does not happen in IE on Windows, firefox on any platform I have tried (Windows, Linux, or OS X), nor 
in Camino.

The problem has occurred since at least version of OS X 10.3 which came with my Mac Mini.

Commerce Bank's only comment on the problem is, "We only support Windows"

[Mark Rowe (bdash)]

A reduced test case for this bug is at http://bdash.net.nz/files/webkit/frames.html

The problem is that the website specifies the frameset rows as "100%*,*", which appears to be treated 
identically to "*,*".

[Niels Leenheer (HTML5test)]

The problem occurs because the parseLength() function checks the last character
and if it is an asterisk (*) it will turn the rest into an integer, regardless
of the percentage sign that is also present.

This means that "100%*,*" is interpreted as "100*,1*"

Solution is to check for an % before converting to an integer. If it is present
ignore the * and return the length as an percentage.

This means that "100%*,*" is interpreted as "100%,1*"

[Niels Leenheer (HTML5test)]

Created attachment 2179 [details]
Check for % and return as percentage

Patch to emulate IE behavoir.

[Niels Leenheer (HTML5test)]

Created attachment 2182 [details]
Check for % and return as percentage (fixed)

Fixed coding style

[Niels Leenheer (HTML5test)]

While creating a layout test for this problem I discovered a related problem.
Not only will "100%*" give problems, so does "100*%".

The cause is probably the toInt() function:
return Length(QConstString(s, l-1).string().toInt(), Percent);

If 's' is "100*%", then 'QConstString.string()' will contain "100*". 
This is the value that is passed to toInt(), which will not output
100, but some other integer.

So, to solve this completely we need to make sure the string passed to 
toInt does not contain % or *. In other words, the patch must be rewritten.

[Niels Leenheer (HTML5test)]

Created attachment 2304 [details]
ignore superfluous * or % in parseLength

Changed the behavoir of the previous patch.

100%* is interpreted as 100%
100*% is interpreted as 100*

This behavoir is consistant with IE/Win

[Niels Leenheer (HTML5test)]

Created attachment 2305 [details]
Testcase for 100%* and 100*%

[Niels Leenheer (HTML5test)]

I've withdrawn this patch from review for now, because I am working with a KHTML
developer to rewrite the length parsing routines completely.

The goal of these efforts are:
- valid HTML should be parsed according to the specs
- invalid HTML should be interpreted in the same way as IE

The new algorithms are already written and are currently being tested on 
my own build of Konqueror. All tests have been succesfull so far, but I did
discover a different bug in frame handling. I will open a new bug report for
it.

[Niels Leenheer (HTML5test)]

Created attachment 2426 [details]
Uber-testcase for all kinds of invalid lenghts

This testcase tests for all kinds of invalid lenghts. 

The first four columns are for testing if fractional lenghts are supported.
There are three possibilities:
- All four columns are different, fracational lenghts are 
  fully supported. 
- The first two are the same and the last two are the same, 
  fractional lenghts are rounded to an integer.
- All four columns are the same, fractional lenghts are 
  floored to an integer.

The remaining columns are grouped in pairs. The first column of a pair contains
the invalid lenghts, the second is a control column which tells us how IE
interprets the invalid lenght preceding it. So, when viewed in IE, all pairs
should be rendered the same.

If the testcase is viewed in a different browser, such as Safari and the two
columns of a pairs are different from each other, this is a signal that the
interpretation of that other browser is different from IE.

[Niels Leenheer (HTML5test)]

Created attachment 2427 [details]
Improved length parser for rows and cols

[Niels Leenheer (HTML5test)]

Created attachment 2429 [details]
Improved length parser for rows and cols

Fixed typo

[Joost de Valk (AlthA)]

Patch works for me, can't say anything about coding style of course :), Niels will attach a screenshot 
showing IE rendering. There's still a small issue, regarding decimals, new bug will be opened for this.

[Niels Leenheer (HTML5test)]

New bug opened for handling of decimal fractions in frame lengths: #3591

[Niels Leenheer (HTML5test)]

Created attachment 2433 [details]
Screenshot of IE behavoir

[Maks Orlovich]

A bit of background/rationale: http://bugs.kde.org/show_bug.cgi?id=76530  
(http://www.giosycento.it/) shows that websites use stuff like <frameset  
cols="22 %,*"> to mean 22%,*. So in particular, splitting on spaces is wrong  
for framesets. It's right for areas though, where there is stuff like  
coords="1 2 3 4" (or coords="1,2 3,4") in the wild.  
  

[Darin Adler]

Comment on attachment 2429 [details]
Improved length parser for rows and cols

Change looks good to me. We must make sure we add plenty of test cases to the
layout tests.

r=me

[Niels Leenheer (HTML5test)]

Created attachment 2490 [details]
Improved length parser for rows and cols

Minor change to previous patch. Instead of skipping over spaces first and then
the remaining digits, skipping of spaces is now done last.

The result extra space are not only ignored with whole numbers, also fractional
decimals: '100.00 %' is now correctly recognized as '100%'

[Niels Leenheer (HTML5test)]

Created attachment 2491 [details]
layout-tests\fast\frames\invalid.html

This file can be placed in:
layout-tests\fast\frames\invalid.html

[Niels Leenheer (HTML5test)]

Created attachment 2492 [details]
layout-tests\fast\frames\valid.html

This file can be placed in:
layout-tests\fast\frames\valid.html

[Niels Leenheer (HTML5test)]

Created attachment 2493 [details]
layout-tests\fast\frames\invalid-expected.txt

[Niels Leenheer (HTML5test)]

Created attachment 2494 [details]
layout-tests\fast\frames\valid-expected.txt

[Niels Leenheer (HTML5test)]

Comment on attachment 2490 [details]
Improved length parser for rows and cols

Request for review. Only a single minor change and addition of layout-tests
(seperately attached)

[Darin Adler]

Comment on attachment 2490 [details]
Improved length parser for rows and cols

r=me

[Joost de Valk (AlthA)]

verified. Since Niels doesn't have a mac and thus can't verify, i'll do it for him :)