RESOLVED INVALID 34743
[Chromium] Implement WebFrameImpl::numberOfPages
https://bugs.webkit.org/show_bug.cgi?id=34743
Summary [Chromium] Implement WebFrameImpl::numberOfPages
Shinichiro Hamaji
Reported 2010-02-09 02:42:56 PST
This is necessary to implement layoutTestController.numberOfPages in test_shell. This API was added in http://trac.webkit.org/changeset/54533 Note that this is similar to Bug 34471, but a different API.
Attachments
Patch v1 (2.65 KB, patch)
2010-02-09 02:49 PST, Shinichiro Hamaji
no flags
Shinichiro Hamaji
Comment 1 2010-02-09 02:49:06 PST
Created attachment 48398 [details] Patch v1
Adam Barth
Comment 2 2010-02-09 12:16:05 PST
Comment on attachment 48398 [details] Patch v1 LGTM, but fishd should look at his change because it touches the WebKit API.
Darin Fisher (:fishd, Google)
Comment 3 2010-02-09 12:46:14 PST
Comment on attachment 48398 [details] Patch v1 > +++ b/WebKit/chromium/public/WebFrame.h > @@ -490,6 +490,11 @@ public: > float pageWidthInPixels, > float pageHeightInPixels) const = 0; > > + // Returns the number of pages to be printed. > + // This method is used to support layout tests. > + virtual int numberOfPages(float pageWidthInPixels, > + float pageHeightInPixels) const = 0; Since this is about printing, should this be moved into the "printing" section of WebFrame? I realize that you probably stuck this nearby pageNumberForElementById intentionally, and that was probably put next to counterValueForElementById because they are only used by layout tests. Is it necessary to call printBegin before using this function? Would it be advantageous to do so? If we wanted to have a print preview system, perhaps we'd want to know how many pages will be generated, so this API might be nice outside the context of layout tests. At any rate, it seems like we should either make these features of the printing API, or we should create a new section in WebFrame that is dedicated to providing utilities for layout tests. One more question: is it necessary to be able to specify pageWidth and pageHeight in fractions of a pixel? Do these need to be floats, or can they be integers? -Darin
Shinichiro Hamaji
Comment 4 2010-02-10 06:03:03 PST
Thanks for your review! I checked printBegin and found printBegin and printEnd are doing the same stuff as PrintContext::numberOfPages (printBegin returns the number of pages). So, I think we don't need to add a new API. We can implement layoutTestController.numberOfPages only with printBegin and printEnd. I'll close this bug. > At any rate, it seems like we should either make these features of the > printing API, or we should create a new section in WebFrame that is > dedicated to providing utilities for layout tests. It seems there are 3 functions for layout tests (renderTreeAsText, counterValueForElementById, and pageNumberForElementById). I'll open another bug (or re-title this bug) to create a section for them. By the way, both counterValueForElementById and pageNumberForElementById were putted by me and you are right, I put them here because I wanted to put them nearby renderTreeAsText. > One more question: is it necessary to be able to specify pageWidth > and pageHeight in fractions of a pixel? Do these need to be floats, > or can they be integers? I made them float just because PrintContext takes float parameters. As I don't think we'll want to use float sized pages from layout tests, printBegin, which takes WebSize (a struct which has two integers), is sufficient to implement layoutTestController.numberOfPages.
Note You need to log in before you can comment on or make changes to this bug.