Bug 49166 - [Qt] QWebElement::appendInside() doesn't work on head elements
Summary: [Qt] QWebElement::appendInside() doesn't work on head elements
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P5 Minor
Assignee: Nobody
URL:
Keywords: Qt, QtTriaged
Depends on:
Blocks:
 
Reported: 2010-11-08 03:17 PST by Bernhard Rosenkraenzer
Modified: 2010-11-22 02:00 PST (History)
3 users (show)

See Also:


Attachments
Test case (1.00 KB, application/octet-stream)
2010-11-08 03:19 PST, Bernhard Rosenkraenzer
no flags Details
Patch (1.85 KB, patch)
2010-11-21 15:40 PST, Jan Erik Hanssen
no flags Details | Formatted Diff | Diff
Patch (2.84 KB, patch)
2010-11-21 21:02 PST, Jan Erik Hanssen
no flags Details | Formatted Diff | Diff
Patch (2.55 KB, patch)
2010-11-22 01:39 PST, Jan Erik Hanssen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Bernhard Rosenkraenzer 2010-11-08 03:17:46 PST
Using QWebElement::appendInside() on the QWebElement representing the HTML's <head> tag does nothing.

e.g. this works:

QWebElement e=frame->findFirstElement("body");
e.appendInside("<p>test test test</p>");

But this doesn't:

QWebElement e=frame->findFirstElement("head");
e.appendInside("<script type=\"text/javascript\" src=\"test.js\"/>");
Comment 1 Bernhard Rosenkraenzer 2010-11-08 03:19:22 PST
Created attachment 73230 [details]
Test case

Adding simple test case.

FWIW this is with the QtWebKit in the current qt 4.7 branch
Comment 2 Jan Erik Hanssen 2010-11-08 19:04:13 PST
Initial assessment:

HTMLElement::deprecatedCreateContextualFragment() explicitly check if the element is a HEAD element (among others) and does not create a DocumentFragment if that's the case. Explicitly calling Element::deprecatedCreateContextualFragment() instead makes this work.

Not sure yet if this is a Qt documentation issue or an actual bug.
Comment 3 Benjamin Poulain 2010-11-11 08:37:04 PST
(In reply to comment #2)
> Not sure yet if this is a Qt documentation issue or an actual bug.

I think we can consider this as a bug. To create WebKit animation on the fly it can be usefull to add stuff in the <head> element. If this can be supported, that sounds like a nice feature to have.

I give this a low priority.
Comment 4 Jan Erik Hanssen 2010-11-21 15:40:30 PST
Created attachment 74518 [details]
Patch
Comment 5 Andreas Kling 2010-11-21 18:54:05 PST
Comment on attachment 74518 [details]
Patch

Needs an autotest in WebKit/qt/tests/qwebelement
Comment 6 Jan Erik Hanssen 2010-11-21 21:02:04 PST
Created attachment 74528 [details]
Patch
Comment 7 Kenneth Rohde Christiansen 2010-11-22 00:31:19 PST
Comment on attachment 74528 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=74528&action=review

> WebKit/qt/Api/qwebelement.cpp:1030
> +    RefPtr<DocumentFragment> fragment;
>      HTMLElement* htmlElement = static_cast<HTMLElement*>(m_element);
> -    RefPtr<DocumentFragment> fragment = htmlElement->deprecatedCreateContextualFragment(markup);
> +    if (htmlElement->hasLocalName(HTMLNames::headTag))
> +        fragment = htmlElement->Element::deprecatedCreateContextualFragment(markup);
> +    else
> +        fragment = htmlElement->deprecatedCreateContextualFragment(markup);
>  
>      ExceptionCode exception = 0;

Why not just cast it to Element always?
Comment 8 Jan Erik Hanssen 2010-11-22 01:36:59 PST
(In reply to comment #7)
> Why not just cast it to Element always?

Good point. For some reason I thought the default action should be to go through HTMLElement but there's no need for that it seems. I'll upload a new patch.
Comment 9 Jan Erik Hanssen 2010-11-22 01:39:53 PST
Created attachment 74533 [details]
Patch
Comment 10 WebKit Commit Bot 2010-11-22 02:00:01 PST
Comment on attachment 74533 [details]
Patch

Clearing flags on attachment: 74533

Committed r72510: <http://trac.webkit.org/changeset/72510>
Comment 11 WebKit Commit Bot 2010-11-22 02:00:07 PST
All reviewed patches have been landed.  Closing bug.