Skip to content

/ servo

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Opacity not working on span #11172

Open
ocerman opened this issue May 13, 2016 · 11 comments
Open

Opacity not working on span #11172

ocerman opened this issue May 13, 2016 · 11 comments
Labels
A-layout/uncategorized C-has-manual-testcase C-reproduced

Comments

@ocerman
Copy link

@ocerman ocerman commented May 13, 2016

opacity

<!DOCTYPE html>
<html><body style="">
    <span style="opacity: 0.1; font-size: 10em;">X</span>
</body></html>
@broesamle
Copy link
Contributor

@broesamle broesamle commented May 15, 2016

...since my other issue is waiting for another PR to land I would like to have a look here, in between.
@jdm
Copy link
Member

@jdm jdm commented May 15, 2016

@broesamle You're welcome to it! Judging by some quick searching for things like opacity in the layout code, we might not be establishing a stacking context for the span? The code related to might only be used for block flows, not inline ones.
@jdm
Copy link
Member

@jdm jdm commented May 15, 2016

Also https://github.com/servo/servo/wiki/Getting-started-with-layout may be useful.
@broesamle
Copy link
Contributor

@broesamle broesamle commented May 17, 2016

Although I am not yet done with reading through relevant code I expect this one be related to #5910 .
@broesamle
Copy link
Contributor

@broesamle broesamle commented May 20, 2016 •
edited

Thank you for pointing out these docs to me.

quick in-between report:
comparing a minimal example <span> vs <div>, only in the latter case an opacity != 1 is ever returned in fragment::establishes_stacking_context.

First guess: that hints at dom/parsing...
...but let's see
@jdm
Copy link
Member

@jdm jdm commented May 20, 2016

modify_style_for_text might be interfering with the check, in that case.
@broesamle
Copy link
Contributor

@broesamle broesamle commented May 27, 2016

Again, a quick in-between report:
In both exapmles (span and div) I encounter opacity=0.1 in create_fragments_for_node_text_content and modify_style_for_text.

Afterwards, it is no longer present for the span:
And, supporting your suspicion about a lack of stacking contexts, fewer callls to fragment::establishes_stacking_context happen for the span, and none of them has opacity=0.1.

Yet, I will need some more reading/debugging to understand where the opacity info is lost.
@jdm
Copy link
Member

@jdm jdm commented May 27, 2016

I presume that modify_style_for_text is overwriting it. We should see if #11421 addresses this to make sure we're not chasing the same issue.
@broesamle
Copy link
Contributor

@broesamle broesamle commented May 27, 2016

hmm, in my two examples, modify_style_for_text is called exactly once, respectively. in both cases, it sets opacity to 1.0 again, which is not harmful in the span case.

I will wait for the avoid double apply transform has landed and then return to here.
@broesamle
Copy link
Contributor

@broesamle broesamle commented Feb 6, 2017

still the same. I'll give it another try.
image
@nox
Copy link
Member

@nox nox commented Oct 2, 2017

Still true.
@nox nox added the C-reproduced label Oct 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-layout/uncategorized C-has-manual-testcase C-reproduced
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
@jdm @nox @broesamle @ocerman
You can’t perform that action at this time.