Bug 3370 - Page renders in the top portion of the page...
Summary: Page renders in the top portion of the page...
Status: VERIFIED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 412
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Dave Hyatt
URL: https://banking.commercebank.com/
Keywords:
Depends on:
Blocks: 3591
  Show dependency treegraph
 
Reported: 2005-06-08 17:50 PDT by Brian Bartlett
Modified: 2005-06-26 22:58 PDT (History)
1 user (show)

See Also:


Attachments
Check for % and return as percentage (1.13 KB, patch)
2005-06-09 05:45 PDT, Niels Leenheer (HTML5test)
no flags Details | Formatted Diff | Diff
Check for % and return as percentage (fixed) (1.22 KB, patch)
2005-06-09 07:24 PDT, Niels Leenheer (HTML5test)
no flags Details | Formatted Diff | Diff
ignore superfluous * or % in parseLength (2.85 KB, patch)
2005-06-13 14:39 PDT, Niels Leenheer (HTML5test)
no flags Details | Formatted Diff | Diff
Testcase for 100%* and 100*% (413 bytes, text/html)
2005-06-13 14:41 PDT, Niels Leenheer (HTML5test)
no flags Details
Uber-testcase for all kinds of invalid lenghts (2.79 KB, text/html)
2005-06-17 02:47 PDT, Niels Leenheer (HTML5test)
no flags Details
Improved length parser for rows and cols (8.19 KB, patch)
2005-06-17 02:54 PDT, Niels Leenheer (HTML5test)
no flags Details | Formatted Diff | Diff
Improved length parser for rows and cols (8.28 KB, patch)
2005-06-17 06:13 PDT, Niels Leenheer (HTML5test)
darin: review+
Details | Formatted Diff | Diff
Screenshot of IE behavoir (45.16 KB, image/jpeg)
2005-06-17 06:56 PDT, Niels Leenheer (HTML5test)
no flags Details
Improved length parser for rows and cols (8.28 KB, patch)
2005-06-20 03:25 PDT, Niels Leenheer (HTML5test)
darin: review+
Details | Formatted Diff | Diff
layout-tests\fast\frames\invalid.html (1.17 KB, text/html)
2005-06-20 03:28 PDT, Niels Leenheer (HTML5test)
no flags Details
layout-tests\fast\frames\valid.html (1.14 KB, text/html)
2005-06-20 03:29 PDT, Niels Leenheer (HTML5test)
no flags Details
layout-tests\fast\frames\invalid-expected.txt (9.10 KB, text/plain)
2005-06-20 05:06 PDT, Niels Leenheer (HTML5test)
no flags Details
layout-tests\fast\frames\valid-expected.txt (9.10 KB, text/plain)
2005-06-20 05:08 PDT, Niels Leenheer (HTML5test)
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Brian Bartlett 2005-06-08 17:50:27 PDT
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"
Comment 1 Mark Rowe (bdash) 2005-06-08 20:19:03 PDT
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 "*,*".
Comment 2 Niels Leenheer (HTML5test) 2005-06-09 05:43:45 PDT
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*"
Comment 3 Niels Leenheer (HTML5test) 2005-06-09 05:45:56 PDT
Created attachment 2179 [details]
Check for % and return as percentage

Patch to emulate IE behavoir.
Comment 4 Niels Leenheer (HTML5test) 2005-06-09 07:24:18 PDT
Created attachment 2182 [details]
Check for % and return as percentage (fixed)

Fixed coding style
Comment 5 Niels Leenheer (HTML5test) 2005-06-09 15:10:55 PDT
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.

Comment 6 Niels Leenheer (HTML5test) 2005-06-13 14:39:06 PDT
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
Comment 7 Niels Leenheer (HTML5test) 2005-06-13 14:41:26 PDT
Created attachment 2305 [details]
Testcase for 100%* and 100*%
Comment 8 Niels Leenheer (HTML5test) 2005-06-16 04:09:42 PDT
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.
Comment 9 Niels Leenheer (HTML5test) 2005-06-17 02:47:05 PDT
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.
Comment 10 Niels Leenheer (HTML5test) 2005-06-17 02:54:48 PDT
Created attachment 2427 [details]
Improved length parser for rows and cols
Comment 11 Niels Leenheer (HTML5test) 2005-06-17 06:13:26 PDT
Created attachment 2429 [details]
Improved length parser for rows and cols

Fixed typo
Comment 12 Joost de Valk (AlthA) 2005-06-17 06:35:33 PDT
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.
Comment 13 Niels Leenheer (HTML5test) 2005-06-17 06:55:02 PDT
New bug opened for handling of decimal fractions in frame lengths: #3591
Comment 14 Niels Leenheer (HTML5test) 2005-06-17 06:56:27 PDT
Created attachment 2433 [details]
Screenshot of IE behavoir
Comment 15 Maks Orlovich 2005-06-18 07:06:36 PDT
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.  
  
Comment 16 Darin Adler 2005-06-18 16:58:31 PDT
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
Comment 17 Niels Leenheer (HTML5test) 2005-06-20 03:25:21 PDT
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%'
Comment 18 Niels Leenheer (HTML5test) 2005-06-20 03:28:24 PDT
Created attachment 2491 [details]
layout-tests\fast\frames\invalid.html

This file can be placed in:
layout-tests\fast\frames\invalid.html
Comment 19 Niels Leenheer (HTML5test) 2005-06-20 03:29:49 PDT
Created attachment 2492 [details]
layout-tests\fast\frames\valid.html

This file can be placed in:
layout-tests\fast\frames\valid.html
Comment 20 Niels Leenheer (HTML5test) 2005-06-20 05:06:05 PDT
Created attachment 2493 [details]
layout-tests\fast\frames\invalid-expected.txt
Comment 21 Niels Leenheer (HTML5test) 2005-06-20 05:08:06 PDT
Created attachment 2494 [details]
layout-tests\fast\frames\valid-expected.txt
Comment 22 Niels Leenheer (HTML5test) 2005-06-20 05:11:44 PDT
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)
Comment 23 Darin Adler 2005-06-20 08:53:50 PDT
Comment on attachment 2490 [details]
Improved length parser for rows and cols

r=me
Comment 24 Joost de Valk (AlthA) 2005-06-26 22:58:59 PDT
verified. Since Niels doesn't have a mac and thus can't verify, i'll do it for him :)