[webkit-reviews] review denied: [Bug 67093] [Qt] Default window.alert shows HTML entities in certain cases : [Attachment 143208] Patch with fix for comment #2

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed May 23 13:04:26 PDT 2012


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

Attachment 143208: Patch with fix for comment #2
https://bugs.webkit.org/attachment.cgi?id=143208&action=review

------- Additional Comments from Simon Hausmann <hausmann at webkit.org>
I think the patch itself is correct, but there's one little glitch that I think
should be fixed before landing. The current order of things is

    setText(msg)
    setTextFormat(Qt::PlainText);

In theory the order of setting properties shouldn't matter, and in fact in
terms of correctness it won't make a difference here. But in terms of
performance a setText(msg) call with text that contains tags will with QLabel
under the hood result in the allocation of a relatively heavy QTextControl
instance. If you call setTextFormat(Qt::PlainText) _before_ calling setText,
then we can avoid the creation and sub-sequent deletion of the QTextControl
inside.


More information about the webkit-reviews mailing list