assignToHTMLImageElement no longer exists in Qt5
Created attachment 195855 [details] Patch
QWebKit allows developers to export QImage objects to Javascript and use Javascript to assign the QImage to an <img/> element. This is documented in http://qt-project.org/doc/qt-5.0/qtwebkit/qtwebkit-bridge.html The problem is that the documentation no longer matches reality. The (Javascript) function was indeed called assignToHTMLImageElement in Qt 4.8, but in Qt5 it's simply 'assignTo'. The responsible code can be found in qtwebkit/Source/WebCore/bridge/qt/qt_pixmapruntime.cpp (QtPixmapRuntime::getClassRef()). I feel very strongly that there was no good reason to change the function name so rather than updating the documentation to reflect the code the code should simply be changed back to expose the old name, or to allow both. It should be trivial to allow both assignTo and assignToHTMLImageElement to be used. See also https://bugreports.qt-project.org/browse/QTBUG-30208
Comment on attachment 195855 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=195855&action=review > Source/WebCore/ChangeLog:3 > + assignToHTMLImageElement no longer exists in Qt5 We usually add a [Qt] prefix before the titles and ChangeLog headlines of Qt-specific WebKit bugs. > Source/WebCore/bridge/qt/qt_pixmapruntime.cpp:257 > + { "assignToHTMLImageElement", assignToHTMLImageElement, 0 }, > { "assignTo", assignToHTMLImageElement, 0 }, This was a mistake so I think that we can just replace the "assignTo" entry instead of having two names for the function.
I chose to keep the old name so things wouldn't break for anyone using qt-5.0.1 with javascript code using 'assignTo'. I'll submit a new patch to fix both remarks.
Created attachment 196172 [details] Patch
Comment on attachment 196172 [details] Patch Great :)
(In reply to comment #4) > I chose to keep the old name so things wouldn't break for anyone using qt-5.0.1 with javascript code using 'assignTo'. That also makes sense. Since the documentation wasn't matching normallly they should be as skeptical as you were (that is if they bothered digging in the code) and they shouldn't be surprised that it gets changed back. I'll pick this change into 5.1.
Comment on attachment 196172 [details] Patch Rejecting attachment 196172 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', '--bot-id=gce-cq-02', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 196172, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue Last 500 characters of output: t/git/webkit-commit-queue/Source/WebKit/chromium/v8 --revision 14075 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' 55>At revision 14075. ________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' ________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' Updating webkit projects from gyp files... Full output: http://webkit-commit-queue.appspot.com/results/17334471
Comment on attachment 196172 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=196172&action=review > Source/WebCore/ChangeLog:8 > + No new tests (OOPS!). Please state that this bug is already covered by the tst_hybridPixmap API test (which doesn't seem to be run by the bot for some reason). The OOPS in the "Reviewed by" line will be replaced by the commit queue.
Created attachment 196202 [details] Patch
Comment on attachment 196202 [details] Patch Clearing flags on attachment: 196202 Committed r147539: <http://trac.webkit.org/changeset/147539>
All reviewed patches have been landed. Closing bug.