Summary: | Align window.scroll() / scrollTo() / scrollBy() with the CSSOM specification | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eunmi Lee <enmi.lee> | ||||||
Component: | DOM | Assignee: | Chris Dumez <cdumez> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | cdumez, cgarcia, commit-queue, darin, esprehn+autocc, kondapallykalyan, rniwa, simon.fraser, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
URL: | https://drafts.csswg.org/cssom-view/#extensions-to-the-window-interface | ||||||||
Attachments: |
|
Description
Eunmi Lee
2016-05-12 23:03:08 PDT
The bug report, as is stated, does not seem correct as per the spec: https://drafts.csswg.org/cssom-view/#extensions-to-the-window-interface The spec define 2 scroll() functions: void scroll(optional ScrollToOptions options); void scroll(unrestricted double x, unrestricted double y); So if you call scroll() function without parameter, it should call the first one. However, our behavior is incorrect in this case I believe. It looks like we are supposed to use the current's viewport position as x/y but we use 0/0 instead. So I think we should not scroll instead of scrolling to the top-left. Created attachment 278850 [details] WIP patch Fixes the bug on https://members.chosun.com/cms_subscribe/application/index.jsp and aligns us with the spec but needs more testing. It is leveraging Darin's recent support for dictionaries although I had to make it work for floating point typed members. Created attachment 278855 [details]
Patch
Comment on attachment 278855 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=278855&action=review > Source/WebCore/bindings/js/JSDOMConvert.h:35 > +template<typename T, typename Enable = void> struct Converter; I’m not familiar with this technique. > Source/WebCore/page/DOMWindow.cpp:1522 > + RefPtr<FrameView> view = m_frame->view(); Why is this RefPtr? Why not auto*? I guess this was copied from the function below, but I don’t think it needs to be. Comment on attachment 278855 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=278855&action=review >> Source/WebCore/bindings/js/JSDOMConvert.h:35 >> +template<typename T, typename Enable = void> struct Converter; > > I’m not familiar with this technique. Found on http://en.cppreference.com/w/cpp/types/enable_if I did not want to have to specialize for both float and double. Comment on attachment 278855 [details] Patch Clearing flags on attachment: 278855 Committed r200907: <http://trac.webkit.org/changeset/200907> All reviewed patches have been landed. Closing bug. I've surprised at quick patch. Thank you very much Chris! :) Also need to match the spec for element scrolling: bug 161610. |