Bug 40841 – [Qt] Node that have both an opacity and a transform animation on them seem not to fire

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Sam Magnuson]

I have attached a manual test case that reproduces the problem on my page as well as a proposed fix. The timing is not correct either, but I will submit that as a separate bug once I determine what is going on there.

[Sam Magnuson]

Created attachment 59122 [details]
Proposed patch (with testcase)

[Sam Magnuson]

Created attachment 59123 [details]
Test case tar'd up.

[Noam Rosenthal]

Ah, that explains a few things I was seeing :)
Good patch.

[Sam Magnuson]

Created attachment 59438 [details]
Rediff against trunk

[Ariya Hidayat]

Sam, why the patch does not include the test?

[Kenneth Rohde Christiansen]

Comment on attachment 59438 [details]
Rediff against trunk

WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:634
 +              else if (!m_state.drawsContent && m_layer->drawsContent())
Is this an important differencial?

WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:1194
 +      virtual AnimatedPropertyID getAnimatedProperty() const = 0;
We normally do not use get. Maybe just call it animatedProperty()

[Eric Seidel (no email)]

Are there plans to update this patch?

[Sam Magnuson]

(In reply to comment #6)
> (From update of attachment 59438 [details])
> WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:634
>  +              else if (!m_state.drawsContent && m_layer->drawsContent())
> Is this an important differencial?
> 

For the duration of this function, yes. It sync's the two at the end of the function but this conditional just notices that the state has changed and what action to take.

> WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:1194
>  +      virtual AnimatedPropertyID getAnimatedProperty() const = 0;
> We normally do not use get. Maybe just call it animatedProperty()

Fair enough, will rename.

[Sam Magnuson]

Created attachment 71509 [details]
Patch

[Noam Rosenthal]

Sam, this looks like a good patch - would you like it committed or are you waiting for another patch?

[WebKit Commit Bot]

Comment on attachment 71509 [details]
Patch

Rejecting patch 71509 from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', 'build-and-test', '--no-clean', '--no-update', '--test', '--non-interactive']" exit_code: 2
Last 500 characters of output:
ests/websocket/tests/workers ......
http/tests/workers .....
http/tests/xhtmlmp .
http/tests/xmlhttprequest ...........................................................................................................................................................................
http/tests/xmlhttprequest/web-apps ...............
http/tests/xmlhttprequest/workers .........
547.40s total testing time

21631 test cases (99%) succeeded
1 test case (<1%) was new
11 test cases (<1%) had stderr output

Full output: http://queues.webkit.org/results/4615058

[Eric Seidel (no email)]

Comment on attachment 71509 [details]
Patch

An updated patch is needed which includes -expected.txt files.

[Andreas Kling]

*** Bug 59198 has been marked as a duplicate of this bug. ***

[Andreas Kling]

I'll generate the missing -expected.txt and land this.

[Andreas Kling]

Committed r86680: <http://trac.webkit.org/changeset/86680>

[Ademar Reis]

Revision r86680 cherry-picked into qtwebkit-2.2 with commit 4495597 <http://gitorious.org/webkit/qtwebkit/commit/4495597>