RESOLVED FIXED 34777
[Qt] Make possible Qt DRT get a page number for element by ID
https://bugs.webkit.org/show_bug.cgi?id=34777
Summary [Qt] Make possible Qt DRT get a page number for element by ID
Diego Gonzalez
Reported 2010-02-09 15:04:42 PST
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
Attachments
Proposed patch (7.96 KB, patch)
2010-02-09 15:18 PST, Diego Gonzalez
no flags
Proposed patch (7.96 KB, patch)
2010-02-10 04:52 PST, Diego Gonzalez
no flags
Diego Gonzalez
Comment 1 2010-02-09 15:18:52 PST
Created attachment 48445 [details] Proposed patch
WebKit Review Bot
Comment 2 2010-02-09 15:21:04 PST
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.
Antonio Gomes
Comment 3 2010-02-09 20:18:29 PST
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.
Diego Gonzalez
Comment 4 2010-02-10 04:52:12 PST
Created attachment 48487 [details] Proposed patch Checked style
Kenneth Rohde Christiansen
Comment 5 2010-02-10 04:55:31 PST
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?
Diego Gonzalez
Comment 6 2010-02-10 05:09:00 PST
(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.
WebKit Commit Bot
Comment 7 2010-02-10 11:26:55 PST
Comment on attachment 48487 [details] Proposed patch Clearing flags on attachment: 48487 Committed r54612: <http://trac.webkit.org/changeset/54612>
WebKit Commit Bot
Comment 8 2010-02-10 11:27:04 PST
All reviewed patches have been landed. Closing bug.
Antonio Gomes
Comment 9 2010-02-10 19:01:50 PST
*** Bug 34573 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.