Skip to content
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

HTMLImageElement::matches_environment doesn't look quite right. #21587

Open
emilio opened this issue Sep 3, 2018 · 6 comments
Open

HTMLImageElement::matches_environment doesn't look quite right. #21587

emilio opened this issue Sep 3, 2018 · 6 comments
Labels
A-content/dom Interacting with the DOM from web content

Comments

@emilio
Copy link
Member

emilio commented Sep 3, 2018

It's passing ParsingMode::all(), which doesn't really seem like it wants to pass, it should probably pass DEFAULT instead.

This should be testable, I think in Servo the sizes attribute will incorrectly work with min-width: <integer> or such.

@emilio
Copy link
Member Author

emilio commented Sep 3, 2018

I added a couple FIXMEs for this in #21588

@emilio
Copy link
Member Author

emilio commented Sep 3, 2018

cc @nupurbaghel

@nupurbaghel
Copy link
Contributor

So finally it has been solved then ?

@jdm
Copy link
Member

jdm commented Sep 4, 2018

It has not been solved.

@jdm jdm added the A-content/dom Interacting with the DOM from web content label Sep 17, 2018
@nupurbaghel
Copy link
Contributor

nupurbaghel commented Sep 26, 2018

I tried using min-width, max-width queries as well as different combinations of width of images in srcset and according to my understanding, what is happening for now is-

  • any following src is ignored (as mentioned in algo step 4.3)
  • in case any query is not matched (which will be true if we are using min-width: <integer> value eliminating the units), then the max-width image is selected from the srcset.
    The behavior is the same for both parse mode : all() & DEFAULT

As I currently do not realize well how do the 2 parse modes actually differ, any ideas on how to write a test which will give differing tests on both of these will be really beneficial.

Just for reference, the iframe below with the mentioned img tag displays a red image -

<iframe id="frame1" style="width:150px; height:150px" src="......">
</iframe>
<img srcset="/images/red-16x16.png 96w,
            /images/blue96x96.png 16w"
     sizes="(min-width: 200) 96px,
            (min-width: 100) 16px"
     src="/images/green-256x256.png">

@emilio
Copy link
Member Author

emilio commented Sep 26, 2018

So, ParsingMode::all() is effectively ALLOW_UNITLESS_LENGTH | ALLOW_ALL_NUMERIC_VALUES.

I think we're very lax with the parsing of sizes, but (min-width: 1) will produce an always-matching media query now, and should produce a never-matching media query instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-content/dom Interacting with the DOM from web content
Projects
None yet
Development

No branches or pull requests

3 participants