Bug 79313 – IsSynchronizingStyleAttributeFlag could be purged

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Hajime Morrita]

It looks this flag can be implemented in another way to save a bit.

[Hajime Morrita]

Created attachment 128389 [details]
Patch

[Hajime Morrita]

Kling, Antti, What do you think about this?

[Antti Koivisto]

Comment on attachment 128389 [details]
Patch

I don't see why this static needs be in Node. It should be pushed down to StyledElement. Even better would be figuring out how to get rid of this mechanism completely.

[Hajime Morrita]

Created attachment 128608 [details]
Patch

[Hajime Morrita]

(In reply to comment #3)
> I don't see why this static needs be in Node. It should be pushed down to StyledElement. Even better would be figuring out how to get rid of this mechanism completely.
Right. I pushed down the flag to StyledElement. 
At this time, I yet found no good way to eliminate this flag entirely.
Could you take another look?

[Philippe Normand]

Comment on attachment 128608 [details]
Patch

Attachment 128608 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11600516

[Hajime Morrita]

Created attachment 128620 [details]
Patch

[Andreas Kling]

Seems like it would be even cleaner to remove the flag and devise a way to update the style attribute without causing attributeChanged() & friends to be called.

[Hajime Morrita]

Comment on attachment 128620 [details]
Patch

Hmm. Okay, I'll investigate later. Clearing r? for now.

[Andreas Kling]

Created attachment 129127 [details]
Patch

Alternate approach.

[WebKit Review Bot]

Attachment 129127 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/dom/ElementAttributeData.h:72:  The parameter name "name" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Hajime Morrita]

Comment on attachment 129127 [details]
Patch

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

> Source/WebCore/dom/ElementAttributeData.cpp:205
> +

It's sad to have this kind of duplication.
How about to move Element::setAttributeInternal() here to ElementAttributeData::setAttribute() and pass an element for invoking callbacks?
So that we can pass it a null to suppress callbacks and we'd no longer need this code duplication.

[Hajime Morrita]

Comment on attachment 129127 [details]
Patch

r- for pinging purpose. Feel free to turn r? again. It looks we need to fix style error anyway.

[Andreas Kling]

Created attachment 129519 [details]
Patch v2

Add a 'notifyChanged' parameter to setAttribute() instead to minimize copypasta.

[Andreas Kling]

Comment on attachment 129519 [details]
Patch v2

Clearing flags on attachment: 129519

Committed r109285: <http://trac.webkit.org/changeset/109285>

[Andreas Kling]

All reviewed patches have been landed.  Closing bug.