Bug 30628 – [Qt] It is impossible to list the attributes of QWebElement

• Component: WebKit Qt
• Status: RESOLVED FIXED

[Benjamin Poulain]

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.

[Tor Arne Vestbø]

Defer this to 4.7

[Tor Arne Vestbø]

Some Maemo guys were asking about this for testability

[Petri Kiiskinen]

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.

[Antonio Gomes]

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 (?)

[Antonio Gomes]

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();

[Holger Freyther]

Maybe typedef the QPair<QString, QString> to something...

[Kenneth Rohde Christiansen]

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;

[Benjamin Poulain]

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.

[Kenneth Rohde Christiansen]

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

[Tor Arne Vestbø]

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

[Tor Arne Vestbø]

Comment on attachment 42749 [details]
Add a method to list the attributes for a given namespace

> +    QStringList attributesNames;

Same, attributeNames

[Benjamin Poulain]

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

[Kenneth Rohde Christiansen]

(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);

[Benjamin Poulain]

Created attachment 42776 [details]
Add a method to list the attributes for a given namespace

Damn coding style. Thanks Kenneth for noticing.

[WebKit Commit Bot]

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>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.