Implement pageNumberForElementById() method in Qt DRT LayoutTestController, to make Qt DRT able to get page number. LayoutTests: printing/page-break-always.html printing/pageNumerForElementById.html printing/css2.1/page-break-before-000.html printing/css2.1/page-break-after-000.html printing/css2.1/page-break-after-004.html printing/css2.1/page-break-before-001.html printing/css2.1/page-break-after-001.html printing/css2.1/page-break-after-002.html printing/css2.1/page-break-before-002.html printing/css2.1/page-break-inside-000.html
Created attachment 48445 [details] Proposed patch
Attachment 48445 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebKitTools/DumpRenderTree/qt/LayoutTestControllerQt.h:151: Missing spaces around = [whitespace/operators] [4] WebKitTools/DumpRenderTree/qt/LayoutTestControllerQt.cpp:462: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Ignoring "WebKit/qt/Api/qwebframe.cpp": this file is exempt from the style guide. Total errors found: 2 If any of these errors are false positives, please file a bug against check-webkit-style.
great catch ! (In reply to comment #2) > Attachment 48445 [details] did not pass style-queue: > > Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 > WebKitTools/DumpRenderTree/qt/LayoutTestControllerQt.h:151: Missing spaces > around = [whitespace/operators] [4] pls fix this. > WebKitTools/DumpRenderTree/qt/LayoutTestControllerQt.cpp:462: Tests for > true/false, null/non-null, and zero/non-zero should all be done without > equality comparisons. [readability/comparison_to_zero] [5] well, generally the bot is right, but in this specific case we are comparing |width| and |height| with '0', which makes the code easier to read for me than using "!width", so i would not bother much , unless someone (reviewer) thinks different. otherwise, lgtm.
Created attachment 48487 [details] Proposed patch Checked style
Comment on attachment 48487 [details] Proposed patch > > +int QWEBKIT_EXPORT qt_drt_pageNumberForElementById(QWebFrame* qFrame, const QString& id, float width, float height) > +{ > + Frame* frame = QWebFramePrivate::core(qFrame); > + if (!frame) I would have used frame and coreFrame :-) > +int LayoutTestController::pageNumberForElementById(const QString& id, float width, float height) > +{ > + // If no size specified, webpage viewport size is used > + if (!width && !height) { > + width = m_drt->webPage()->viewportSize().width(); > + height = m_drt->webPage()->viewportSize().height(); > + } What is with is 0, but height is not?
(In reply to comment #5) > (From update of attachment 48487 [details]) > > > > > +int QWEBKIT_EXPORT qt_drt_pageNumberForElementById(QWebFrame* qFrame, const QString& id, float width, float height) > > +{ > > + Frame* frame = QWebFramePrivate::core(qFrame); > > + if (!frame) > > I would have used frame and coreFrame :-) humm I just followed the pattern of the others qt_drt* > > +int LayoutTestController::pageNumberForElementById(const QString& id, float width, float height) > > +{ > > + // If no size specified, webpage viewport size is used > > + if (!width && !height) { > > + width = m_drt->webPage()->viewportSize().width(); > > + height = m_drt->webPage()->viewportSize().height(); > > + } > > What is with is 0, but height is not? I assumed the method will calculate using 0 for width and a given value for height, and it will possibly return a invalid value for that.
Comment on attachment 48487 [details] Proposed patch Clearing flags on attachment: 48487 Committed r54612: <http://trac.webkit.org/changeset/54612>
All reviewed patches have been landed. Closing bug.
*** Bug 34573 has been marked as a duplicate of this bug. ***