RESOLVED FIXED 85692
Convert isPageBoxVisible to use Internals interface
https://bugs.webkit.org/show_bug.cgi?id=85692
Summary Convert isPageBoxVisible to use Internals interface
Gyuyoung Kim
Reported 2012-05-04 19:38:44 PDT
Adjust isPageBoxVisible tests to use Internals instead of LayoutTestController interface. In my humble opinion, isPageBoxVisible() is able to be supported by Internals. Because, it looks this function is implemented by WebCore layer, not WebKit layer. I'd like to know how do reviewers think about this patch.
Attachments
Patch (30.00 KB, patch)
2012-05-04 20:51 PDT, Gyuyoung Kim
no flags
Patch (30.01 KB, patch)
2012-05-04 23:51 PDT, Gyuyoung Kim
no flags
Patch (31.19 KB, patch)
2012-05-05 06:53 PDT, Gyuyoung Kim
no flags
Gyuyoung Kim
Comment 1 2012-05-04 20:51:19 PDT
Early Warning System Bot
Comment 2 2012-05-04 21:05:51 PDT
WebKit Review Bot
Comment 3 2012-05-04 21:17:23 PDT
Comment on attachment 140379 [details] Patch Attachment 140379 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12543527
Build Bot
Comment 4 2012-05-04 21:20:05 PDT
Build Bot
Comment 5 2012-05-04 21:47:04 PDT
Early Warning System Bot
Comment 6 2012-05-04 22:10:10 PDT
Gyuyoung Kim
Comment 7 2012-05-04 23:51:09 PDT
Build Bot
Comment 8 2012-05-05 00:18:34 PDT
Gyuyoung Kim
Comment 9 2012-05-05 06:53:37 PDT
Darin Adler
Comment 10 2012-05-05 17:22:34 PDT
Comment on attachment 140397 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=140397&action=review > Tools/DumpRenderTree/mac/LayoutTestControllerMac.mm:-315 > - return [mainFrame isPageBoxVisible:pageNumber]; I can’t speak for the other ports, but for the Mac port, the isPageBoxVisible: method exists only to support this DumpRenderTree feature. So we should also be removing that method from WebKit entirely.
WebKit Review Bot
Comment 11 2012-05-06 15:42:33 PDT
Comment on attachment 140397 [details] Patch Clearing flags on attachment: 140397 Committed r116246: <http://trac.webkit.org/changeset/116246>
WebKit Review Bot
Comment 12 2012-05-06 15:42:41 PDT
All reviewed patches have been landed. Closing bug.
Andy Estes
Comment 13 2012-05-06 17:29:13 PDT
This patch broke the build on my machine due to leaving behind an unused function. I removed the function in <http://trac.webkit.org/changeset/116252>.
Gyuyoung Kim
Comment 14 2012-05-06 17:38:22 PDT
(In reply to comment #13) > This patch broke the build on my machine due to leaving behind an unused function. I removed the function in <http://trac.webkit.org/changeset/116252>. Thank you for your fix for build break.
Note You need to log in before you can comment on or make changes to this bug.