[webkit-reviews] review requested: [Bug 67093] [Qt] Default window.alert shows HTML entities in certain cases : [Attachment 143039] Patch to fix the display of alert(), confirm() and prompt()

[bugzilla-daemon at webkit.org]

Steffen Imhof <steffen.imhof at basyskom.com> has asked  for review:
Bug 67093: [Qt] Default window.alert shows HTML entities in certain cases
https://bugs.webkit.org/show_bug.cgi?id=67093

Attachment 143039: Patch to fix the display of alert(), confirm() and prompt()
https://bugs.webkit.org/attachment.cgi?id=143039&action=review

------- Additional Comments from Steffen Imhof <steffen.imhof at basyskom.com>
The problem is caused by the HTML escaping introduced in
https://bugs.webkit.org/show_bug.cgi?id=34429, which in turn was added to make
sure HTML tags are shown in the alert boxes. The root cause for me seem to be
Qt's text detection automatisms. So the text is sometimes interpreted as plain
text and sometimes as rich text (with a few possible HTML tags).
In my opinion the correct solution here is to force plain text. (As far as I
can tell, inside the message boxes this should not pose any security risk?!)
The static convenience methods cannot be used anymore in this case, so the code
grew a bit.
QInputDialog does not provide an API to access the contained label, so I had to
resort to some "less-than-nice" meta object inspection for "prompt()". A
possible alternative would be to write our own dialog for this case, but I
think this would be overkill.
A somewhat related problem is fixed with this patch, too: QLabel interprets '&'
characters as the start of a mnemonic identifier, dropping the '&' and
underlining the next character. This is worked around by doubling the '&'s
beforehand.
I haven't seen yet, how to test this automatically, but I don't think a test
should be necessary for this fix. (If I am wrong here, I'd be glad for some
pointers how one would write such a test case.)