Summary: | [Qt] Need a public API for suspending/resuming DOM Objects on a page | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jesus Sanchez-Palencia <jesus> | ||||||||||
Component: | WebKit Qt | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED WONTFIX | ||||||||||||
Severity: | Normal | CC: | hausmann, kenneth, kent.hansen, kling, webkit.review.bot | ||||||||||
Priority: | P2 | Keywords: | Qt, QtTriaged | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Attachments: |
|
Description
Jesus Sanchez-Palencia
2010-01-19 12:43:09 PST
Created attachment 46941 [details]
Proposed API for QWebFrame and QWebPage, and manual-tests
Created attachment 46942 [details]
Add an option to QtLauncher to be easy to suspend/resume DOM Objects on the current page
Comment on attachment 46941 [details]
Proposed API for QWebFrame and QWebPage, and manual-tests
Do we have license to distribute Elvis_with_parachute?
Attachment 46942 [details] did not build on qt: Build output: http://webkit-commit-queue.appspot.com/results/198987 (In reply to comment #3) >Do we have license to distribute Elvis_with_parachute? The place I took it said that it was "free for using" and that you should contact the author for any specific questions about license. I just can't find the author, so it's probably better to use another GIF from our resources. I'll fix this. (In reply to comment #4) > Attachment 46942 [details] did not build on qt: > Build output: http://webkit-commit-queue.appspot.com/results/198987 Attachment 46942 [details] depends on attachment 46941 [details]. First of all it is called Qt and not QT. QT == QuickTime Comment on attachment 46941 [details] Proposed API for QWebFrame and QWebPage, and manual-tests > > The API from QWebPage will call suspend/resume DOM Objects of the main frame > and of all its children. Add API to QWebPage so that it is possible to suspend/resume... > * manual-tests/frames-multiple-active-dom-objects.html: Added. > * manual-tests/resources/Elvis_with_parachute.gif: Added. What is the license of this file? > +// FIXME: add support to suspending/resuming of animated GIFs and plugins. > +// Suspend DOM objects (JS timers, CSS animation, SVG animation) in this frame. > +bool QWebFramePrivate::suspendDOMObjects() > +{ > + if (!frame) > + return false; > + > + AnimationController* controller = frame->animation(); > + if (!controller) > + return false; This seems wrong... what if you dont have a controller, can't you still suspend the active DOM objects included by doc->suspendActiveDOMObjects(); ? > + > + Document* doc = frame->document(); > + if (!doc) > + return false; > + > + // JavaScript > + doc->suspendActiveDOMObjects(); > +bool QWebFramePrivate::resumeDOMObjects() same thing here > + QList<QWebFrame *> subFrames = frame->childFrames(); Should be QList<QWebFrame*>. childFrames is probably a better name than subFrames :-) and it is already used > + QList<QWebFrame *>::const_iterator i; > + for (i = subFrames.constBegin(); i != subFrames.constEnd() ; ++i) { > + b = (b && suspendChildFrames((*i))); > + } > + Don't use b, use a descriptive name. > +bool QWebPagePrivate::resumeChildFrames(QWebFrame *frame) Wrong coding style, QWebFrame* frame > +{ > + if (!frame) > + return false; > + > + bool b = frame->resumeDOMObjects(); > + Don't use b > + QList<QWebFrame *> subFrames = frame->childFrames(); > + QList<QWebFrame *>::const_iterator i; Coding style > +/*! > + Resume all frames, starting at the main frame. > + DOM Objects (JS timers, CSS and SVG animation) will be resumed. DOM objects that were suspended, will be resumed. > + bool suspendChildFrames(QWebFrame *frame); > + bool resumeChildFrames(QWebFrame *frame); Coding style Comment on attachment 46942 [details] Add an option to QtLauncher to be easy to suspend/resume DOM Objects on the current page > From debee88b204b8faf48edca5bca37b6b25e5a26cc Mon Sep 17 00:00:00 2001 > From: Jesus Sanchez-Palencia <jesferna@darjeeling.(none)> > Date: Tue, 19 Jan 2010 17:17:45 -0300 > Subject: [PATCH 2/2] Add suspend/resume page option to QtLauncher > > Reviewed by NOBODY (OOPS!). > > [QT] Need a public API for suspending/resuming DOM Objects on a page > https://bugs.webkit.org/show_bug.cgi?id=33857 It is called Qt > + suspendDOMObjects = false; Doesn't the launcher uses m_ for member fields? > + QWebPage* page = view->page(); > + page->suspend(); Why not call it directly? > + void toggleSuspendResumeObjects(bool on) That doesnt seem much like a toggle if it takes a bool argument. Created attachment 47026 [details]
[v2] Proposed API and manual-tests
Created attachment 47027 [details]
[v2] Add a suspend/resume option to QtLauncher
This patch depends on the other attachment (47026).
Attachment 47027 [details] did not build on qt: Build output: http://webkit-commit-queue.appspot.com/results/204430 Comment on attachment 47026 [details] [v2] Proposed API and manual-tests Clearing review for the time being, given that we're waiting for more feedback on the requirement at http://bugreports.qt.nokia.com/browse/QTWEBKIT-5 Comment on attachment 47027 [details]
[v2] Add a suspend/resume option to QtLauncher
Clearning review here, too
Please follow the QtWebKit bug reporting guidelines when reporting bugs. See http://trac.webkit.org/wiki/QtWebKitBugs Specifically: - The 'QtWebKit' component should only be used for bugs/features in the public QtWebKit API layer, not to signify that the bug is specific to the Qt port of WebKit http://trac.webkit.org/wiki/QtWebKitBugs#Component - Add the keyword 'Qt' to signal that it's a Qt-related bug http://trac.webkit.org/wiki/QtWebKitBugs#Keywords I think Jesus needs a reply at http://bugreports.qt.nokia.com/browse/QTWEBKIT-5 in order for this to proceed. Closing this bug as a long time ago it was decided this API wasn't needed. |