WebKit Bugzilla
You need to log in before you can comment on or make changes to this bug.
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 an attachment (id=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] [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 an attachment (id=48487) [details] Proposed patch Checked style
(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 :-) > +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] [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.
(From update of attachment 48487 [details]) 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. ***