WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Proposed patch
(7.96 KB, patch)
2010-02-10 04:52 PST
,
Diego Gonzalez
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
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.
Top of Page
Format For Printing
XML
Clone This Bug