Closed Bug 1419221 Opened 3 years ago Closed 3 years ago

Make getUnanimatedComputedStyle and test cases for getUnanimatedComputedStyle more robust

Categories

(Core :: DOM: Animation, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: hiro, Assigned: hiro)

References

Details

Attachments

(3 files)

Currently DOMWindowUtils.getUnanimatedComputedStyle does not flush any pending styles, but the test case for the function does not flush pending style before calling the function [1]

So the test cases have been passed accidentally.  I did discussed with Brian on IRC, we might want to add an enum to the function for flush styles explicitly.

Also the test case for pseudo element is also broken since we call getUnanimatedComputedStyle() with '::before' before 'content: none' property is applied. 

[1] https://hg.mozilla.org/mozilla-central/file/081c06e175b2/dom/base/test/file_domwindowutils_animation.html#l119
(In reply to Hiroyuki Ikezoe (:hiro) from comment #0)
> Also the test case for pseudo element is also broken since we call
> getUnanimatedComputedStyle() with '::before' before 'content: none' property
> is applied. 

I mean 'content: ""' of course.
Summary: Make test cases for getUnanimatedComputedStyle more robust → Make getUnanimatedComputedStyle and test cases for getUnanimatedComputedStyle more robust
It seems that IPDL does not support enum type yet (as far as I can tell no interface implements enum type), so I am going to reuse FLUSH_STYLE there [1] and add "FLUSH_NONE = -1".

[1] https://hg.mozilla.org/mozilla-central/file/081c06e175b2/dom/interfaces/base/nsIDOMWindowUtils.idl#l993
Assignee: nobody → hikezoe
Comment on attachment 8930358 [details]
Bug 1419221 - Create element for the target of getUnanimatedComputedStyle().

https://reviewboard.mozilla.org/r/201494/#review206730

Nice find.
Attachment #8930358 - Flags: review?(bbirtles) → review+
Comment on attachment 8930359 [details]
Bug 1419221 - Generate target pseudo element before calling getUnanimatedComputedStyle for the pseudo element.

https://reviewboard.mozilla.org/r/201496/#review206736

(That test is so hard to follow. I would love to rewrite it one day.)
Attachment #8930359 - Flags: review?(bbirtles) → review+
Comment on attachment 8930360 [details]
Bug 1419221 - Introduce a new argument for getUnanimatedComputedStyle to be able to flush pending styles.

https://reviewboard.mozilla.org/r/201498/#review206740

::: dom/base/test/file_domwindowutils_animation.html:130
(Diff revision 1)
> +    () => utils.getUnanimatedComputedStyle(div, null, "opacity", utils.FLUSH_DISPLAY),
> +    "NS_ERROR_INVALID_ARG",
> +    "FLUSH_DISPLAY option should throw");
> +
> +  if (utils.isStyledByServo) {
> +    // Flush styles since getUnanimatedComputedStyle flushes pending styled even

s/styled/styles/ ?

::: dom/base/test/file_domwindowutils_animation.html:141
(Diff revision 1)
> +       "1",
> +       "getUnanimatedComputedStyle with FLUSH_NONE should not flush pending styles");
> +
> +    is(utils.getUnanimatedComputedStyle(div, null, "opacity", utils.FLUSH_STYLE),
> +       "0",
> +       "getUnanimatedComputedStyle with FLUS_STYLE should flush pending styles");

FLUSH_STYLE
Attachment #8930360 - Flags: review?(bbirtles) → review+
Pushed by hikezoe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0931a288f6e0
Create element for the target of getUnanimatedComputedStyle(). r=birtles
https://hg.mozilla.org/integration/autoland/rev/b096c6518c9f
Generate target pseudo element before calling getUnanimatedComputedStyle for the pseudo element. r=birtles
https://hg.mozilla.org/integration/autoland/rev/696edf3997a4
Introduce a new argument for getUnanimatedComputedStyle to be able to flush pending styles. r=birtles
You need to log in before you can comment on or make changes to this bug.