Bug 61132

Summary: fix render overflow computation for input type=range
Product: WebKit Reporter: Tony Chang <tony>
Component: New BugsAssignee: Tony Chang <tony>
Status: RESOLVED FIXED    
Severity: Normal CC: ademar, hyatt, jamesr, mitz
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 59644    
Attachments:
Description Flags
Patch
none
Patch
none
Patch jamesr: review+

Description Tony Chang 2011-05-19 10:48:07 PDT
fix render overflow computation for input type=range
Comment 1 Tony Chang 2011-05-19 10:51:56 PDT
Created attachment 94089 [details]
Patch
Comment 2 Tony Chang 2011-05-19 11:11:20 PDT
Created attachment 94092 [details]
Patch
Comment 3 Tony Chang 2011-05-19 11:11:55 PDT
(In reply to comment #2)
> Created an attachment (id=94092) [details]
> Patch

I moved the call to m_overflow.clear() to be outside the thumb check.  It's possible for the thumb to be remove and the overflow not cleared.
Comment 4 James Robinson 2011-05-19 13:44:20 PDT
Comment on attachment 94092 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=94092&action=review

Test is (theoretically at least) flaky

> LayoutTests/fast/forms/slider-hit-testing.html:1
> +<head>

<!DOCTYPE html> plz unless this test needs quirks mode

> LayoutTests/fast/forms/slider-hit-testing.html:11
> +window.setTimeout(function() {
> +  document.getElementById("slider").style.width = "100%";
> +  changeAndCheckRangeValue();
> +}, 0);

the setTimeout is a bit odd - does the test not work if you run this code in an onload handler? technically speaking, this setTimeout _could_ execute immediately after parsing the end of the <script> and before the <input> is parsed, although in practice we'll nearly always parse through the end of this document before running timeouts.

> LayoutTests/fast/forms/slider-hit-testing.html:20
> +    var x = slider.offsetLeft + (slider.clientWidth * .75);
> +    var y = slider.offsetTop + (slider.clientHeight / 2);

could you add some comments indicating what you are trying to click on here and what should be happening?
Comment 5 Tony Chang 2011-05-19 14:07:17 PDT
Created attachment 94112 [details]
Patch
Comment 6 Tony Chang 2011-05-19 14:08:23 PDT
Comment on attachment 94092 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=94092&action=review

>> LayoutTests/fast/forms/slider-hit-testing.html:1
>> +<head>
> 
> <!DOCTYPE html> plz unless this test needs quirks mode

Done.

>> LayoutTests/fast/forms/slider-hit-testing.html:11
>> +}, 0);
> 
> the setTimeout is a bit odd - does the test not work if you run this code in an onload handler? technically speaking, this setTimeout _could_ execute immediately after parsing the end of the <script> and before the <input> is parsed, although in practice we'll nearly always parse through the end of this document before running timeouts.

I didn't work in the onload handler, but I added a call to document.body.offsetLeft to force a layout.  This allowed me to remove the waitUntilDone/notifyDone calls.

>> LayoutTests/fast/forms/slider-hit-testing.html:20
>> +    var y = slider.offsetTop + (slider.clientHeight / 2);
> 
> could you add some comments indicating what you are trying to click on here and what should be happening?

Done.
Comment 7 James Robinson 2011-05-23 19:24:58 PDT
Comment on attachment 94112 [details]
Patch

Looks good!
Comment 8 Tony Chang 2011-05-24 10:44:04 PDT
Committed r87168: <http://trac.webkit.org/changeset/87168>
Comment 9 Ademar Reis 2011-05-24 14:01:49 PDT
Revision r87168 cherry-picked into qtwebkit-2.2 with commit 49b8a1e <http://gitorious.org/webkit/qtwebkit/commit/49b8a1e>