The document.nodesFromRect() function should never have been added to Document.idl since that makes it a web facing API. We should remove it ASAP and instead add WebKit level API as necessary. For ports that want this functionality exposed, a good place might be on their WebView. (NOTE: if it is determined that there are good use cases for this feature, we can add it back, prefixed with webkit).
sam, any chance we can get bug 46336 (Qt needs this change), while I work on that one? So right after bug 46336 gets in, I can: (quick action plan) 1) remove the method from the IDL; 2) Skip the layout test relies on it to be in the IDL. (once _1_ and _2_ are done right way I can:) 3) make it testable through LayoutTestController and unskip the test again. What do you think?
Created attachment 68827 [details] (committed with r69345, r=kenneth,kling) patch v1 Patch removes nodesFromRect() from Document.idl, skips the test relying on it, and fixes the window-properties-expected.txt accordingly.
Comment on attachment 68827 [details] (committed with r69345, r=kenneth,kling) patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=68827&action=review > LayoutTests/ChangeLog:9 > + Skip the tests relying on document.nodesFromRect in all platform for > + the moment, and adjusting the expected results tests also accordingly. Are you going to readd these tests later with DRT support?
(In reply to comment #3) > (From update of attachment 68827 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=68827&action=review > > > LayoutTests/ChangeLog:9 > > + Skip the tests relying on document.nodesFromRect in all platform for > > + the moment, and adjusting the expected results tests also accordingly. > > Are you going to readd these tests later with DRT support? Yes, it will be LayoutTestController::nodesFromRect(document, x, y, topPadding, rightPading, ...). Filing bugs for each platform now. Thanks.
Comment on attachment 68827 [details] (committed with r69345, r=kenneth,kling) patch v1 Clearing review flag on attachment: 68827. Committed r68345: <http://trac.webkit.org/changeset/68345>
Oh, nice idea. Can we add that to webkit-patch?
(In reply to comment #6) > Oh, nice idea. Can we add that to webkit-patch? That one of the main reasons I am not using webkit-patch to land patches, and we even discussed that briefly in the past: http://www.mail-archive.com/webkit-dev@lists.webkit.org/msg10090.html , including you pointing out where to look at in the code :)
Revision r68345 cherry-picked into qtwebkit-2.1 with commit 659ae6b <http://gitorious.org/webkit/qtwebkit/commit/659ae6b>