1295852 - elements named _moz_generated_content_before/after should not be considered generated content containers

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Cameron McCormack (:heycam)]

There are a few places where we compare the name atom of an element against nsGkAtoms::mozgeneratedcontentbefore/after.  We really should also be checking IsInNativeAnonymousSubtree(), since regular content can use the _moz_generated_content_before/after names too.

Comment 1

[Cameron McCormack (:heycam)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d9f32107ac96

Comment 2

[Cameron McCormack (:heycam)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b3327dd30ae5

Comment 3

[Cameron McCormack (:heycam)]

Attachment: Bug 1295852 - Part 1: Add nsINode methods for testing whether a node is a ::before/::after generated content container.

Review commit: https://reviewboard.mozilla.org/r/72448/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/72448/

Comment 4

[Cameron McCormack (:heycam)]

Attachment: Bug 1295852 - Part 2: Check for generated content containers correctly in Element.getAnimations().

Review commit: https://reviewboard.mozilla.org/r/72450/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/72450/

Comment 5

[Cameron McCormack (:heycam)]

Attachment: Bug 1295852 - Part 3: Check for generated content containers correctly when generating DOM ranges from a11y selections.

Review commit: https://reviewboard.mozilla.org/r/72452/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/72452/

Comment 6

[Cameron McCormack (:heycam)]

Attachment: Bug 1295852 - Part 4: Check for generated content containers more precisely when reflowing grid containers.

I'm not convinced that we can actually get regular content
<_moz_generated_content_before> elements in here, since it's only if
CompareTreePosition returned 0 that we look at the element name,
but might be best to make this test stronger in any case.

Review commit: https://reviewboard.mozilla.org/r/72454/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/72454/

Comment 7

[Cameron McCormack (:heycam)]

Attachment: Bug 1295852 - Part 5: Check for generated content containers more precisely in nsTransitionManager assertion.

Review commit: https://reviewboard.mozilla.org/r/72456/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/72456/

Comment 8

[Boris Chiou [:boris]]

Comment on attachment 8782254 [details]
Bug 1295852 - Part 2: Check for generated content containers correctly in Element.getAnimations().

https://reviewboard.mozilla.org/r/72450/#review70050

Comment 9

[Boris Chiou [:boris]]

Comment on attachment 8782257 [details]
Bug 1295852 - Part 5: Check for generated content containers more precisely in nsTransitionManager assertion.

https://reviewboard.mozilla.org/r/72456/#review70052

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Away: -9/28)]

Comment on attachment 8782255 [details]
Bug 1295852 - Part 3: Check for generated content containers correctly when generating DOM ranges from a11y selections.

https://reviewboard.mozilla.org/r/72452/#review70066

I don't know why you request to review me this patch, though, it looks good to me.

Comment 11

[Cameron McCormack (:heycam)]

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #10)
> I don't know why you request to review me this patch, though, it looks good
> to me.

Blame told me you wrote that code.  Thanks for the review.

Comment 12

[Mats Palmgren (inactive)]

Comment on attachment 8782256 [details]
Bug 1295852 - Part 4: Check for generated content containers more precisely when reflowing grid containers.

https://reviewboard.mozilla.org/r/72454/#review70264

Why move the IsPrevContinuationOf inside the MOZ_[UN]LIKELY?
If no reason other than indentation, then I prefer the current way
since it has higher fidelity about which part is [un]likely.
r=mats

Comment 13

[Bobby Holley (:bholley)]

Comment on attachment 8782253 [details]
Bug 1295852 - Part 1: Add nsINode methods for testing whether a node is a ::before/::after generated content container.

https://reviewboard.mozilla.org/r/72448/#review70318

Comment 14

[Cameron McCormack (:heycam)]

(In reply to Mats Palmgren (:mats) from comment #12)
> Why move the IsPrevContinuationOf inside the MOZ_[UN]LIKELY?
> If no reason other than indentation, then I prefer the current way
> since it has higher fidelity about which part is [un]likely.

That's a mistake!  I'll it back out of there.

Comment 15

[Cameron McCormack (:heycam)]

Comment on attachment 8782256 [details]
Bug 1295852 - Part 4: Check for generated content containers more precisely when reflowing grid containers.

https://reviewboard.mozilla.org/r/72454/#review70520

(Turning Mats' r=me / Bugzilla r+ into a mozreview r+.)

Comment 16

[Cameron McCormack (:heycam)]

Comment on attachment 8782253 [details]
Bug 1295852 - Part 1: Add nsINode methods for testing whether a node is a ::before/::after generated content container.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/72448/diff/1-2/

Comment 17

[Cameron McCormack (:heycam)]

Comment on attachment 8782254 [details]
Bug 1295852 - Part 2: Check for generated content containers correctly in Element.getAnimations().

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/72450/diff/1-2/

Comment 18

[Cameron McCormack (:heycam)]

Comment on attachment 8782255 [details]
Bug 1295852 - Part 3: Check for generated content containers correctly when generating DOM ranges from a11y selections.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/72452/diff/1-2/

Comment 19

[Cameron McCormack (:heycam)]

Comment on attachment 8782256 [details]
Bug 1295852 - Part 4: Check for generated content containers more precisely when reflowing grid containers.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/72454/diff/1-2/

Comment 20

[Cameron McCormack (:heycam)]

Comment on attachment 8782257 [details]
Bug 1295852 - Part 5: Check for generated content containers more precisely in nsTransitionManager assertion.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/72456/diff/1-2/

Comment 21

[Cameron McCormack (:heycam)]

Comment on attachment 8782256 [details]
Bug 1295852 - Part 4: Check for generated content containers more precisely when reflowing grid containers.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/72454/diff/2-3/

Comment 22

[Cameron McCormack (:heycam)]

Comment on attachment 8782257 [details]
Bug 1295852 - Part 5: Check for generated content containers more precisely in nsTransitionManager assertion.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/72456/diff/2-3/

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/14d293a0db97
Part 1: Add nsINode methods for testing whether a node is a ::before/::after generated content container. r=bholley
https://hg.mozilla.org/integration/autoland/rev/acee0d460e39
Part 2: Check for generated content containers correctly in Element.getAnimations(). r=boris
https://hg.mozilla.org/integration/autoland/rev/25e6c15f9431
Part 3: Check for generated content containers correctly when generating DOM ranges from a11y selections. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/a62d860735be
Part 4: Check for generated content containers more precisely when reflowing grid containers. r=me
https://hg.mozilla.org/integration/autoland/rev/b70e0aba06af
Part 5: Check for generated content containers more precisely in nsTransitionManager assertion. r=boris

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/14d293a0db97
https://hg.mozilla.org/mozilla-central/rev/acee0d460e39
https://hg.mozilla.org/mozilla-central/rev/25e6c15f9431
https://hg.mozilla.org/mozilla-central/rev/a62d860735be
https://hg.mozilla.org/mozilla-central/rev/b70e0aba06af

Comment 25

[Mats Palmgren (inactive)]

Comment on attachment 8782256 [details]
Bug 1295852 - Part 4: Check for generated content containers more precisely when reflowing grid containers.

https://reviewboard.mozilla.org/r/72454/#review70702

Comment 26

[Mats Palmgren (inactive)]

(In reply to Cameron McCormack (:heycam) from comment #15)
> (Turning Mats' r=me / Bugzilla r+ into a mozreview r+.)

Sorry about that; it appears that if you set the pref dom.storage.enabled
to false like I do, then the "Review state" combobox is absent in the new
ReviewBoard UI.  (filed bug 1296662)