Bug 29639 – [Qt] Update the stypeProperty API of QWebElement.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Qt

[Jocelyn Turcotte]

Change to the QWebElement as discussed in meetings+mails.

The ResolveRule vs. StyleResolveStrategy enum name was still under discussion, please add any comment regarding this if needed.

The patch also update the documentation.

[Jocelyn Turcotte]

Created attachment 39916 [details]
Patch

[Kenneth Rohde Christiansen]

I like the new StyleResolveStrategy, but I think we should use the old name for it, the ResolveRule, as it is more a rule than a strategy, as what we are doing is very well defined. You could call it StyleResolveRule or maybe even CssResolveRule?

[Antonio Gomes]

patch does much more than what is described in bug title: "Update QWebElement API to remove QtScript related methods."

could we split it up ? maybe even in two bugs ?

[Eric Seidel (no email)]

Comment on attachment 39916 [details]
Patch

r- based on the above request to have this split.  28k is pretty big for a patch, making it difficult to review. :(

[Jocelyn Turcotte]

Created attachment 40058 [details]
Split patch

Yes this makes sense, I've split the patch in 3 including these two other bugs:
https://bugs.webkit.org/show_bug.cgi?id=29708
https://bugs.webkit.org/show_bug.cgi?id=29709

This patch now only contain the modifications about QWebElement::styleProperty:

(QWebElement::styleProperty):
- Merge the stypeProperty and the computedStyleProperty methods
- Remove the default value for the style resolving enum
- Rename ResolveRule to StyleResolveStrategy
(QWebElement::setStyleProperty):
- Remove the priority argument since it is possible to control the
  behaviour by adding !important or removing in the value.

[Jocelyn Turcotte]

Created attachment 40060 [details]
Split patch v0.2

- Removes the extra computedStyle method from qwebelement.h
- Specify in setStyleProperty's doc that this modifies the inline style.

[Antonio Gomes]

jocelyn, changes look nice to me, as they make api simpler.

one simple question: should we keep the "\since 4.6" ?

(..)
-    \enum QWebElement::ResolveRule
-    \since 4.6
+    \enum QWebElement::StyleResolveStrategy
(..)

[Jocelyn Turcotte]

In fact QWebElement in whole is new to 4.6 so it is not needed.
I took the opportunity of this patch to change this, I should have noted this extra change earlier in the bug or changelog.

(In reply to comment #7)
> jocelyn, changes look nice to me, as they make api simpler.
> 
> one simple question: should we keep the "\since 4.6" ?
> 
> (..)
> -    \enum QWebElement::ResolveRule
> -    \since 4.6
> +    \enum QWebElement::StyleResolveStrategy
> (..)

[Simon Hausmann]

Comment on attachment 40060 [details]
Split patch v0.2

r=me. Let's discuss Strategy vs. Rule with Kenneth though, we can still change it

[Simon Hausmann]

Committed r48749: <http://trac.webkit.org/changeset/48749>