RFC 71: Support adding URI fragments for reftests #71
Conversation
|
|
||
| ## Details | ||
|
|
||
| A text fragment url for highlighting text in a page can be done with fragments |
There was a problem hiding this comment.
Is text-fragments the only motivating use case here? It would be a stronger proposal with more use cases than just a WCIG spec with (afaict) limited vendor interest.
There was a problem hiding this comment.
Is text-fragments the only motivating use case here? It would be a stronger proposal with more use cases than just a WCIG spec with (afaict) limited vendor interest.
The highlight styling is in a CSSWG draft which is what I was writing tests for:
https://drafts.csswg.org/css-pseudo-4/#selectordef-target-text
I don't know about any other use cases atm, assuming the meta support wouldn't be more convenient than:
<script>window.location.hash = "#target";</script>
for other fragments.
There was a problem hiding this comment.
I think one could set window.location.hash for other fragments, but that's not really intuitive (e.g. subtleties like screenshot timing -- Does this trick delay load? / Is reftest-wait needed?), and the semantics are not exactly the same.
The proposed implementation here seems pretty straightforward and is a natural generalization of the existing variants system, so I think the convenience/intuitiveness alone is well worth the effort, let alone the specific use case unblocked by it.
|
|
||
| ## Risks | ||
|
|
||
| * There is a question what it means to have multiple variants for ref-tests. |
There was a problem hiding this comment.
I think this proposal needs to answer these questions. In particular the question around the behaviour of refs with variants is one reason we didn't support reftests with variants before now.
I don't have a strong sense of what the answer should be, but there are at least several possibilities:
- ref urls are unaffected. This means that all variants of the test need to have identical rendering so it's probably only useful for the case where there's a single variant but it happens to contain a query or fragment part.
- ref urls always get the same variant as the test. This means the ref has to do some work to ensure the rendering is correct given the variant. This might not work in cases where it's hard to adjust the reference rendering without relying on the feature that's being tested.
- The variant syntax is extended to allow different behaviour for refs and tests. This is the most complex option, but also the one with the most flexibility.
There was a problem hiding this comment.
I don't have strong opinions here.
There was a problem hiding this comment.
To be clear I don't think you have to answer this question, but I do think getting agreement on a design is essential to progressing the RFC.
There was a problem hiding this comment.
I think the first two are both reasonable. I'd slightly prefer the first, which is more in line with the principle of keeping refs as simple as possible.
There was a problem hiding this comment.
Option 1 is the easiest (and we can lint for a reftest only having 1 variant if we want), but it definitely abuses the variant notation for a purpose that isn't a variant at all (it's just some stuff to stick on the URL).
I'm not sure I see the problem for option 2. I agree that each reference would have to handle the variant (if there are multiple) to produce the correct matching (or mismatching) rendering. However I think that's a specific case of a general problem for any solution that wants to support variants for reftests - i.e. once reftests have variants then the set of listed references cannot be all correct by default (or the variants are usless).
For option 3, I think we need to identify the actual desirable behavior if we want to do that. For example, is it that you want to list multiple refs, but only have some apply for each variant? (And should those refs that apply get passed the variant too?). We could borrow from fuzzy-reftests for that:
<meta name="variant" content="ref-1.html:?1-100">
<meta name="variant" content="ref-2.html:?101-200">
<meta name="variant" content="ref-3.html:?201-300">
<meta name="variant" content="ref-4.html,ref-5.html:?301-400">
But this seems complex and it's hard for me to imagine use cases for it.
Overall I think I'm ok with supporting option 1 as a known 'hack' of the variant syntax for now... again it's hard to tell if that will cause backwards compat problem when we don't know what conditions option 3 would actually be trying to satisfy. (And I don't have a good suggestion for it!)
Looking forward to @jgraham 's thoughts, which I'm sure will be much better than mine :)
There was a problem hiding this comment.
I suppose #2 is probably OK.
The problem I had in mind is if you had some feature that caused the browser to change the default rendering on the basis of the hash or something. e.g. if some #magic=text causes all instances of text to render in yellow. Then it would be useful to not pass that to the ref since you don't actually want the feature to be used on the reference page at all. If you pass it in anyway you rely on the override CSS on the reference page to ensure that the feature itself isn't affecting rendering.
If we wanted to worry about that we could have seperate ref meta items or could do something like <meta name=variant content="ref=//?1-100"> where if the content starts with ref= that gets applied to the ref and we use something like // as the seperator (I'm not sure what the best choice there is).
I guess if we aren't worried about the scenario where default rendering may also be applied to the ref, we should implement option 2 and consider extending this later if we run into problems. It seems pretty forward-compatible to do it like that.
There was a problem hiding this comment.
The problem I had in mind is if you had some feature that caused the browser to change the default rendering on the basis of the hash or something. e.g. if some #magic=text causes all instances of text to render in yellow. Then it would be useful to not pass that to the ref since you don't actually want the feature to be used on the reference page at all. If you pass it in anyway you rely on the override CSS on the reference page to ensure that the feature itself isn't affecting rendering.
Ah yes, that makes sense. But I also agree that we can revisit the idea if needed to account for this sort of scenario.
@lilles - can you own updating the RFC to encompass the behavior from option 2?
|
@jgraham and I discussed this RFC today (apologies for the delay). We agree that the multiple variants problem needs some thought; both of us have committed to think about it and get back to this issue. Please ping me if there isn't an update by next week :) |
|
Thanks for the update |
Ping? |
|
I had forgotten that In this case the ref is the same for all variants. |
* added elements * Fix test to apply background-color to all elements and fix expectations * Test all properties that disable native appearance, and include <a> * Test properties that do not disable native appearance * Test 'revert' keyword does not result in the fallback appearance * Generate tests instead of using non-supported 'variant' * Update pass condition for meter and progress See web-platform-tests/rfcs#71 See w3c/csswg-drafts#6537 (comment) Co-authored-by: Simon Pieters <zcorpan@gmail.com>
Automatic update from web-platform-tests HTML: Compute kind of widget (#30267) * added elements * Fix test to apply background-color to all elements and fix expectations * Test all properties that disable native appearance, and include <a> * Test properties that do not disable native appearance * Test 'revert' keyword does not result in the fallback appearance * Generate tests instead of using non-supported 'variant' * Update pass condition for meter and progress See web-platform-tests/rfcs#71 See w3c/csswg-drafts#6537 (comment) Co-authored-by: Simon Pieters <zcorpan@gmail.com> -- wpt-commits: 34974b0e244cf597e08f08e75f3fccb6d52aec8f wpt-pr: 30267
foolip
changed the title
|
@mrego said in web-platform-tests/wpt#26364 (comment):
So I guess one question is whether we still have any need for this feature? |
|
It should no longer be necessary for my original use case, at least. |
|
Let's go ahead and close this since the original motivation is no longer there. If someone else wants to drive it, feel free to clone into a new RFC to resolve the remaining issues. |
No description provided.