Bug 113639 – [Qt] assignToHTMLImageElement no longer exists in Qt5

• Component: New Bugs
• Status: RESOLVED FIXED

[Kristof Provost]

assignToHTMLImageElement no longer exists in Qt5

[Kristof Provost]

Created attachment 195855 [details]
Patch

[Kristof Provost]

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

[Jocelyn Turcotte]

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.

[Kristof Provost]

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.

[Kristof Provost]

Created attachment 196172 [details]
Patch

[Jocelyn Turcotte]

Comment on attachment 196172 [details]
Patch

Great :)

[Jocelyn Turcotte]

(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.

[WebKit Review Bot]

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

[Jocelyn Turcotte]

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.

[Kristof Provost]

Created attachment 196202 [details]
Patch

[WebKit Review Bot]

Comment on attachment 196202 [details]
Patch

Clearing flags on attachment: 196202

Committed r147539: <http://trac.webkit.org/changeset/147539>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.