Bug 30628 - [Qt] It is impossible to list the attributes of QWebElement
Summary: [Qt] It is impossible to list the attributes of QWebElement
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: Qt
Depends on:
Blocks:
 
Reported: 2009-10-21 06:56 PDT by Benjamin Poulain
Modified: 2009-11-09 12:22 PST (History)
6 users (show)

See Also:


Attachments
Proposal for a patch to allow getting the list of all attributes for a qweblement (1.72 KB, patch)
2009-11-05 12:46 PST, Petri Kiiskinen
kenneth: review-
Details | Formatted Diff | Diff
Add a method to list the attributes for a given namespace (4.10 KB, patch)
2009-11-09 07:05 PST, Benjamin Poulain
vestbo: review-
vestbo: commit-queue-
Details | Formatted Diff | Diff
Add a method to list the attributes for a given namespace (4.09 KB, patch)
2009-11-09 11:26 PST, Benjamin Poulain
no flags Details | Formatted Diff | Diff
Add a method to list the attributes for a given namespace (4.12 KB, patch)
2009-11-09 12:04 PST, Benjamin Poulain
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Benjamin Poulain 2009-10-21 06:56:49 PDT
QWebElement has the method QWebElement::hasAttributes() to know if there is any attributes available, but there is no way to access those attributes if the name is not known.

QDomNode gives access to the list of attributes with "QDomNamedNodeMap QDomNode::attributes()". There is no such things in QWebElement.
Comment 1 Tor Arne Vestbø 2009-10-27 05:36:08 PDT
Defer this to 4.7
Comment 2 Tor Arne Vestbø 2009-10-27 08:32:59 PDT
Some Maemo guys were asking about this for testability
Comment 3 Petri Kiiskinen 2009-11-05 12:46:21 PST
Created attachment 42588 [details]
Proposal for a patch to allow getting the list of all attributes for a qweblement

The patch will create a new public method for QWebElement in order to allow getting a list of all attributes for an element. The patch is tested against gitorious qt repo 4.6 branch allthough it should work on top of other forks as well as it is not touching existing methods - just adding one new.
Comment 4 Antonio Gomes 2009-11-05 14:07:48 PST
Comment on attachment 42588 [details]
Proposal for a patch to allow getting the list of all attributes for a qweblement

i beleive you meant to r? not r+ the patch (?)
Comment 5 Antonio Gomes 2009-11-05 14:14:51 PST
hum, a few comments:


from an API user prospective, I'd find this an unclear API at first glance. It could at least get better documented.

+QMap<QPair<QString,QString>,QString> QWebElement::attributes() const

i'd also splt the line below into two sentenses...

+ attributes[QPair<QString,QString>(attribute->namespaceURI(),attribute->localName())] = 
+                    attribute->value();
Comment 6 Holger Freyther 2009-11-07 00:33:47 PST
Maybe typedef the QPair<QString, QString> to something...
Comment 7 Kenneth Rohde Christiansen 2009-11-09 06:19:09 PST
Comment on attachment 42588 [details]
Proposal for a patch to allow getting the list of all attributes for a qweblement

First of all, I would like to postpone API additions to 4.7, unless you have a good argument for why this is required in 4.6. We have already branched off, so the patch can get committed, but it would be nice to know if we need to cherry-pick this change or not.

> +QMap<QPair<QString,QString>,QString> QWebElement::attributes() const

Yes, maybe a typedef would be nice.

> +{
> +    QMap<QPair<QString,QString>,QString> attributes;
> +    if (m_element) {
> +        NamedNodeMap* attrs = m_element->attributes(true);
> +        if (attrs) {
> +            unsigned numAttrs = attrs->length();
> +            for (unsigned i = 0; i < numAttrs; i++) {
> +                Attribute *attribute = attrs->attributeItem(i);

coding style violation. * should be to the left.

> +                attributes[QPair<QString,QString>(attribute->namespaceURI(),attribute->localName())] = 

coding style: missing space after ,. Please run the check-webkit-style script

> +                    attribute->value();
> +            }
> +        }
> +            
> +    }
> +    return attributes;
> +}
> +
> +
> +/*!
>      Returns true if the element has keyboard input focus; otherwise, returns false
>  
>      \sa setFocus()
> diff --git a/src/3rdparty/webkit/WebKit/qt/Api/qwebelement.h b/src/3rdparty/webkit/WebKit/qt/Api/qwebelement.h
> index 351ccb4..6495cc7 100644
> --- a/src/3rdparty/webkit/WebKit/qt/Api/qwebelement.h
> +++ b/src/3rdparty/webkit/WebKit/qt/Api/qwebelement.h
> @@ -71,6 +71,7 @@ public:
>      void removeAttribute(const QString& name);
>      void removeAttributeNS(const QString& namespaceUri, const QString& name);
>      bool hasAttributes() const;
> +    QMap<QPair<QString,QString>,QString> attributes() const;
>  
>      QStringList classes() const;
>      bool hasClass(const QString& name) const;
Comment 8 Benjamin Poulain 2009-11-09 07:05:50 PST
Created attachment 42749 [details]
Add a method to list the attributes for a given namespace

Here is a patch to list the attributes of a namespace. Getting the list of namespaces available would be in a separate patch.

For me, this changes is for Qt 4.7, it is too late to change the API of 4.6.
Comment 9 Kenneth Rohde Christiansen 2009-11-09 07:44:08 PST
(In reply to comment #8)
> Created an attachment (id=42749) [details]
> Add a method to list the attributes for a given namespace
> 
> Here is a patch to list the attributes of a namespace. Getting the list of
> namespaces available would be in a separate patch.
> 
> For me, this changes is for Qt 4.7, it is too late to change the API of 4.6.

Im fine with that.

+QStringList QWebElement::attributesName(const QString &namespaceUri) const

I would use Url as all of our API talks about Urls and not about Uris. Also that & should be to the left.
Comment 10 Tor Arne Vestbø 2009-11-09 07:49:04 PST
Comment on attachment 42749 [details]
Add a method to list the attributes for a given namespace

> +QStringList QWebElement::attributesName(const QString &namespaceUri) const

Should be attributeNames, names in plural.

The argument-name 'namespaceUri' is fine, that's what we use for QWebElement::attributeNS()
Comment 11 Tor Arne Vestbø 2009-11-09 07:51:23 PST
Comment on attachment 42749 [details]
Add a method to list the attributes for a given namespace

> +    QStringList attributesNames;

Same, attributeNames
Comment 12 Benjamin Poulain 2009-11-09 11:26:23 PST
Created attachment 42774 [details]
Add a method to list the attributes for a given namespace

Changes:
-integrate the remarks of Tor Arne
-const String namespaceUriString(namespaceUri); is inside the if()
Comment 13 Kenneth Rohde Christiansen 2009-11-09 11:34:54 PST
(In reply to comment #12)
> Created an attachment (id=42774) [details]
> Add a method to list the attributes for a given namespace
> 
> Changes:
> -integrate the remarks of Tor Arne
> -const String namespaceUriString(namespaceUri); is inside the if()

There are still coding style violations :-) at least one.

+QStringList QWebElement::attributeNames(const QString &namespaceUri) const

Also why are you making a new string from the string passed in?

namespaceUriString(namespaceUri);
Comment 14 Benjamin Poulain 2009-11-09 12:04:12 PST
Created attachment 42776 [details]
Add a method to list the attributes for a given namespace

Damn coding style. Thanks Kenneth for noticing.
Comment 15 WebKit Commit Bot 2009-11-09 12:22:31 PST
Comment on attachment 42776 [details]
Add a method to list the attributes for a given namespace

Clearing flags on attachment: 42776

Committed r50676: <http://trac.webkit.org/changeset/50676>
Comment 16 WebKit Commit Bot 2009-11-09 12:22:35 PST
All reviewed patches have been landed.  Closing bug.