Bug 98832 - Remove fast/events/attempt-scroll-with-no-scrollbars.html
Summary: Remove fast/events/attempt-scroll-with-no-scrollbars.html
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Julien Chaffraix
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-09 16:13 PDT by Julien Chaffraix
Modified: 2013-02-28 11:16 PST (History)
7 users (show)

See Also:


Attachments
Proposed removal (8.43 KB, patch)
2012-10-15 18:17 PDT, Julien Chaffraix
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Julien Chaffraix 2012-10-09 16:13:56 PDT
The test was landed in http://trac.webkit.org/changeset/28823 and was disabled at the time of landing. 5 years have passed and all platforms are still disabling it.

The change seems to be related to bug 14607 (especially comment 14). Which describes the bug as:

"The actual problem here is that there are no window-level scrollbars drawn (as there are 6 scripts that should be printed in the pop-up menu).  This is covered by:
<rdar://problem/5566435>"

Opening the test, there is a window-level scrollbar drawn but the test is failing so it's unclear if the test is valid.

As no one has undertook to fix this issue, it would be better to remove the test from trunk and if the bug is still valid, file a follow-up bug with the test attached to it. CC'ing some people from the original change so that they can give more context or push back on the idea.
Comment 1 Julien Chaffraix 2012-10-15 18:17:52 PDT
Created attachment 168831 [details]
Proposed removal

I would really love to hear from anyone with more knowledge of the context if there's a better path for this test.
Comment 2 Darin Adler 2013-01-16 15:09:01 PST
Comment on attachment 168831 [details]
Proposed removal

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

> LayoutTests/ChangeLog:10
> +        The test was landed failing in http://trac.webkit.org/changeset/28823 and has been disabled on
> +        all platforms since then. It's unclear now if we should fix the bug (due to the limited amount
> +        of knowledge available nowadays) but keeping this test forever failing doesn't help any port.

I don’t think the “limited knowledge available nowadays” part of this is right.

All there is to the history here, I believe, is that Alice Barraclough discovered that you can scroll a scrollbars=no window with window.scrollTo even though you can’t scroll it with the scrollbars or scroll wheel. She created this test case to demonstrate the problem and filed an internal Apple bug report about it.

Looking back on it, I think it would have been better to have this with expected results showing the failure, rather than as a skipped test. But I also think that we can decide at this point to decide to not treat this as a bug. At one point, Dan Bernstein pointed out that other browsers (at least Firefox 3) allow the window.scrollTo and so it’s not clear it’s a problem that WebKit allows it too.

So I think it’s OK for us to remove this test as part of deciding not to treat it as a bug. Or it would be OK to keep the test and land the expected failure instead of skipping this test on every port. Running the test with an expected result, even one that reflects what is possibly a problem, does have value. It documents our behavior, helps us notice if we change it unintentionally.
Comment 3 Julien Chaffraix 2013-02-28 11:16:05 PST
Comment on attachment 168831 [details]
Proposed removal

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

>> LayoutTests/ChangeLog:10
>> +        of knowledge available nowadays) but keeping this test forever failing doesn't help any port.
> 
> I don’t think the “limited knowledge available nowadays” part of this is right.
> 
> All there is to the history here, I believe, is that Alice Barraclough discovered that you can scroll a scrollbars=no window with window.scrollTo even though you can’t scroll it with the scrollbars or scroll wheel. She created this test case to demonstrate the problem and filed an internal Apple bug report about it.
> 
> Looking back on it, I think it would have been better to have this with expected results showing the failure, rather than as a skipped test. But I also think that we can decide at this point to decide to not treat this as a bug. At one point, Dan Bernstein pointed out that other browsers (at least Firefox 3) allow the window.scrollTo and so it’s not clear it’s a problem that WebKit allows it too.
> 
> So I think it’s OK for us to remove this test as part of deciding not to treat it as a bug. Or it would be OK to keep the test and land the expected failure instead of skipping this test on every port. Running the test with an expected result, even one that reflects what is possibly a problem, does have value. It documents our behavior, helps us notice if we change it unintentionally.

The test didn't really give information about what it's testing. You could obviously infer what was going on from the test case but that's a stretch. Also the only resource to help explain that was the rdar link which is unavailable to most people.

You are right that I should have done my research more thoroughly about whether it was passing on other browsers. I just did and no browser matches the behavior tested by this test case:
* IE & WebKit scrolls to 100.
* Firefox scrolls to 42.
* Opera (FWIW) scrolls to 23.

Based on that, we could change the expectations to assert that we do scroll and treat the original report as a non-bug.