CSS opacity transition broken with parent pseudo :before and overflow auto

RESOLVED FIXED in mozilla31

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: adam.flynn.schwartz, Assigned: mats)

Tracking

28 Branch
mozilla31
x86
macOS
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Reporter

Description

5 years ago
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/33.0.1750.152 Safari/537.36

Steps to reproduce:

The following combination of HTML/CSS produces a non-working CSS transition:

http://jsfiddle.net/adamschwartz/h8Nk7/
http://jsfiddle.net/adamschwartz/h8Nk7/show/light/


Actual results:

The opacity value for the inner span changes immediately on hover.


Expected results:

The opacity value should transition.

Updated

5 years ago
Component: Untriaged → CSS Parsing and Computation
Product: Firefox → Core
Presumably we're reframing here for some reason.  I wonder whether the scrollframe confuses things as to whether the ::before should exist or something.
Assignee

Comment 2

5 years ago
Posted patch wip (obsolete) — Splinter Review
The ReconstructFrame hint is added here:
http://mxr.mozilla.org/mozilla-central/source/layout/base/RestyleManager.cpp?rev=b29352bc495f#2666
It shouldn't be necessary b/c we already have a ::before frame at that
point, nsLayoutUtils::GetBeforeFrame just can't find it.

(This is just a quick hack to demonstrate it works if we fix that
somehow - I'm not suggesting this is the correct fix.)
Assignee: nobody → matspal
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
> nsLayoutUtils::GetBeforeFrame just can't find it

Because it ends up in the scrollbar rathole?

Can you thing of any reason why the ::before frame would not be under the content insertion frame?  Seems to me like this is in fact the right fix, but should be done inside GetBeforeFrame (and GetAfterFrame, I expect).
Assignee

Comment 4

5 years ago
(In reply to Boris Zbarsky [:bz] from comment #3)
> Because it ends up in the scrollbar rathole?

Yes.

> Can you thing of any reason why the ::before frame would not be under the
> content insertion frame?

No, let's make that a requirement from now on.

>  Seems to me like this is in fact the right fix,
> but should be done inside GetBeforeFrame (and GetAfterFrame, I expect).

Yes, that's what I had in mind.
Assignee

Updated

5 years ago
Attachment #8401512 - Attachment is obsolete: true
Summary: CSS opacity transition broken with parent psuedo :before and overflow auto → CSS opacity transition broken with parent pseudo :before and overflow auto
Comment on attachment 8409383 [details] [diff] [review]
Use GetContentInsertionFrame() when searching for ::before/::after frames.

What guarantees that the value will actually be < 100 when the timer fires in the test?  Seems like it could fire late enough that the transition completes.

Why can't we just call testbug990340() sync instead of calling it off a timeout?

r=me on the actual code changes, but would like to get the test bit sorted.
Attachment #8409383 - Flags: review?(bzbarsky) → review+
Also, am I right that this patch fixes the two bugs this blocks?
Flags: needinfo?(matspal)
Assignee

Comment 8

5 years ago
(In reply to Boris Zbarsky [:bz] from comment #6)
> What guarantees that the value will actually be < 100 when the timer fires
> in the test? Seems like it could fire late enough that the transition
> completes.

30 seconds late?

> Why can't we just call testbug990340() sync instead of calling it off a
> timeout?

OK, no problem.
Flags: needinfo?(matspal)
Assignee

Comment 9

5 years ago
(In reply to Boris Zbarsky [:bz] (on PTO, reviews are slow) from comment #7)
> Also, am I right that this patch fixes the two bugs this blocks?

Yes, AFAICT.
> 30 seconds late?

Yes.  We've totally had intermittent oranges on other tests due to timers firing _minutes_ late.
https://hg.mozilla.org/mozilla-central/rev/86119be8c527
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.