Bug 29843

Summary: [Qt] Review all new API in Qt 4.6
Product: WebKit Reporter: Simon Hausmann <hausmann>
Component: WebKit QtAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ariya.hidayat, ben, commit-queue, eric, kenneth, laszlo.gombos, tonikitoo, vestbo, yael
Priority: P2 Keywords: Qt
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 30530, 30630, 30710, 30772, 30773, 31249    
Bug Blocks: 29799    
Attachments:
Description Flags
Rename the fixedContentsSize property
none
First part of the clipContentsToViewport fix (introduce paint method, deprecate render) none

Simon Hausmann
Reported 2009-09-29 03:53:00 PDT
Most of the new API has been reviewed already, but what's missing includes: 2. QWebFrame::requestedUrl() and QwebFrame::baseUrl() 3. Layout and rendering We've added a few methods that relate to layout and rendering. These are: QWebFrame::clipRenderToViewPort() QWebFrame::scrollBarGeometry() QWebFrame::contentSizeChanged() QWebPage::fixedContentSize() QWebView::renderHints() We need to go though these and consider them together + revisit API. 4. QWebFrame::loadStarted() and QWebFrame::loadFinished() 5. Saving and restoring of QWebHistory 6. QWebKitVersion 7. QWebPluginDatabase Needs review, also in the context of not having anything to do with QWebPluginFactory, ie it's only NPAPI. 8. QWebSecurityOrigin QWebSecurityOrigin::add/remove/LocalScheme() QWebSecurityOrigin::whitelistAccesFromOrigin() 9. Storage New settings in QWebSettings Setting of paths Enabling all persistent storage We need to revisit this and make sure the naming and the API is still consistent. 10. Misc added methods QWebPage::frameAt() QWebFrame::focus() QWebPage::shouldInterupJavaScript() QWebView::guessUrlFromString() QWebSettings::defaltTextEncoding() QWebSettings::clearMemoryCached(); 11. QWebGraphicsItem
Attachments
Rename the fixedContentsSize property (7.65 KB, patch)
2009-10-16 13:46 PDT, Kenneth Rohde Christiansen 		no flags
DetailsFormatted DiffDiff
First part of the clipContentsToViewport fix (introduce paint method, deprecate render) (6.32 KB, patch)
2009-10-19 13:31 PDT, Kenneth Rohde Christiansen 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Simon Hausmann
Comment 1 2009-10-13 09:31:14 PDT
We had a quick round of API review with Matthias here at the DevDays and came up with the following suggestions for changes: * QWebFrame::clipContentsToViewport: 1) Remove the property 2) Deprecate QWebFrame::render() 3) Introduce QWebFrame::paint(QWebFrame::PaintOptions opts = QWebFrame::AllElements), so that one can for eaxmple call frame->paint(QWebFrame::Contents); frame->paint(QWebFrame::Contents | QWebFrame::ScrollBars); * QWebFrame::scrollBarGeometry(): good name, keep it * QWebPage::fixedContentsSize property: 1) Rename to preferredContentsSize
Kenneth Rohde Christiansen
Comment 2 2009-10-16 13:46:50 PDT
Created attachment 41320 [details] Rename the fixedContentsSize property
Simon Hausmann
Comment 3 2009-10-16 18:14:52 PDT
Comment on attachment 41320 [details] Rename the fixedContentsSize property Thanks! Please fix the spelling of my first name in the ChangeLog when landing though ;-)
WebKit Commit Bot
Comment 4 2009-10-18 23:51:32 PDT
Comment on attachment 41320 [details] Rename the fixedContentsSize property Clearing flags on attachment: 41320 Committed r49766: <http://trac.webkit.org/changeset/49766>
WebKit Commit Bot
Comment 5 2009-10-18 23:51:36 PDT
All reviewed patches have been landed. Closing bug.
Tor Arne Vestbø
Comment 6 2009-10-19 02:51:40 PDT
Closed by bot, but this is a tracker-bug
Kenneth Rohde Christiansen
Comment 7 2009-10-19 13:31:12 PDT
Created attachment 41443 [details] First part of the clipContentsToViewport fix (introduce paint method, deprecate render)
Kenneth Rohde Christiansen
Comment 8 2009-10-19 13:32:02 PDT
Follow up patch will remove the clipContentsToViewport property
Eric Seidel (no email)
Comment 9 2009-10-19 13:34:24 PDT
Please open a new bug for new fixes. If you want to use tracker bugs, you should open bugs for all the individual fixes and marked them blocked on this one. I think this bug should be closed and the patch moved to a new bug.
Tor Arne Vestbø
Comment 10 2009-10-19 13:42:25 PDT
I agree with Eric, we should have individual blocker-bugs for each patch. But do we really need to delete this bug? Can't we obsolete/remove the patches and just keep on using it to track the API reviews?
Tor Arne Vestbø
Comment 11 2009-10-27 08:47:21 PDT
Some comments after a small session we had in Oslo. The only API left now should be QGraphicsWebView. 1. QWebFrame::loadStarted() and QWebFrame::loadFinished() Do we need these? It's duplicating the problem of the 'ok' argument. 2. QWebKitVersion - Who's going to use this? Are we exposing a version that has no real use? 3. QWebPluginDatabase Propose to make this private. See bug 30775 4. QWebSecurityOrigin::add/remove/LocalScheme() Rename to register*? 5. QWebSecurityOrigin::whitelistAccesFromOrigin() ++ Rename to q_drt (they were added for DRT) 6. Misc QWebPage::shouldInterupJavaScript() - did we decide to remove this? QWebSettings::defaltTextEncoding() - QTextCodec? QWebView::guessUrlFromString()- Remove from WebKit, lives in QUrl now QWebSettings::setPrintingMinimumShrinkFactor etc 4.6 or 4.7?
Antonio Gomes
Comment 12 2009-10-27 08:56:14 PDT
Tor I'd like to request discussion for https://bugs.webkit.org/show_bug.cgi?id=30771 if possible. I sent an email to webkit-qt ML ealier today titled "qweb{page,view} and qgraphicswebview createWindow method."
Kenneth Rohde Christiansen
Comment 13 2009-10-27 10:36:31 PDT
(In reply to comment #11) > Some comments after a small session we had in Oslo. The only API left now > should be QGraphicsWebView. > > 1. QWebFrame::loadStarted() and QWebFrame::loadFinished() > > Do we need these? It's duplicating the problem of the 'ok' argument. I think Yael could answer that. > 2. QWebKitVersion - Who's going to use this? Are we exposing a version that has > no real use? Useful for changing the user agent, you want to set the right webkit version. Also useful for browsers. > Rename to q_drt (they were added for DRT) In that case I agree. > 6. Misc > > QWebPage::shouldInterupJavaScript() - did we decide to remove this? No we did not, but at least there should be a setting to set the timeout. > QWebView::guessUrlFromString()- Remove from WebKit, lives in QUrl now It does? since when? Thiago was opposed to that.
Kenneth Rohde Christiansen
Comment 14 2009-10-27 12:23:59 PDT
5. QWebSecurityOrigin::whitelistAccesFromOrigin() ++ Rename to q_drt (they were added for DRT) DONE. Landed in 50166.
Kenneth Rohde Christiansen
Comment 15 2009-10-27 12:44:23 PDT
> > QWebView::guessUrlFromString()- Remove from WebKit, lives in QUrl now > > It does? since when? Thiago was opposed to that. OK, it was added, but due to license problems it will be removed from Qt by Thiago and readded by Benjamin (icefox). When this has been done I will remove the method. This should happen early tomorrow morning.
Yael
Comment 16 2009-10-29 17:37:32 PDT
(In reply to comment #13) > (In reply to comment #11) > > Some comments after a small session we had in Oslo. The only API left now > > should be QGraphicsWebView. > > > > 1. QWebFrame::loadStarted() and QWebFrame::loadFinished() > > > > Do we need these? It's duplicating the problem of the 'ok' argument. > > I think Yael could answer that. > Looking at svn logs, I think Simon added these signals for DRT tests, These are not the signals that I have added.
Kenneth Rohde Christiansen
Comment 17 2009-11-09 05:29:31 PST
(In reply to comment #16) > (In reply to comment #13) > > (In reply to comment #11) > > > Some comments after a small session we had in Oslo. The only API left now > > > should be QGraphicsWebView. > > > > > > 1. QWebFrame::loadStarted() and QWebFrame::loadFinished() > > > > > > Do we need these? It's duplicating the problem of the 'ok' argument. > > > > I think Yael could answer that. > > > > Looking at svn logs, I think Simon added these signals for DRT tests, These are > not the signals that I have added. Rename to q_drt (they were added for DRT) then.
Simon Hausmann
Comment 18 2009-11-10 06:55:09 PST
Closing this bug as all the API has been reviewed and fixed. We're past the deadline of API changes for Qt 4.6 now.
Note You need to log in before you can comment on or make changes to this bug.