[CSS21][CSS22] Seeking review of position:fixed edge case test

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[CSS21][CSS22] Seeking review of position:fixed edge case test

Chris Rebert
Hi folks,

I'm trying to get a reviewer for a CSS2 regression test:
https://github.com/w3c/csswg-test/pull/813

Basically, WebKit doesn't currently handle position:fixed correctly
when it's also top,bottom,left,right:auto and a child of a
position:relative element. The test asserts the spec'd behavior which
Chrome, Firefox, IE11, and Presto adhere to.

Thanks,
Chris
--
Bootstrap Core Team member
Browser 🐛s galore: http://getbootstrap.com/browser-bugs/


Reply | Threaded
Open this post in threaded view
|

Re: [CSS21][CSS22] Seeking review of position:fixed edge case test

GĂ©rard Talbot-3
Le 2015-07-27 03:21, Chris Rebert a écrit :

> Hi folks,
>
> I'm trying to get a reviewer for a CSS2 regression test:
> https://github.com/w3c/csswg-test/pull/813
>
> Basically, WebKit doesn't currently handle position:fixed correctly
> when it's also top,bottom,left,right:auto and a child of a
> position:relative element. The test asserts the spec'd behavior which
> Chrome, Firefox, IE11, and Presto adhere to.
>
> Thanks,
> Chris


Chris,

Your test is a good test. Indeed iPhone 6, Safari 7+ fail your test.
Here's some review comments.


1- There is already a test with the filename position-fixed-001

http://test.csswg.org/suites/css2.1/nightly-unstable/html4/position-fixed-001.htm

in the test suite. Fortunately for you, your test, in my opinion, is
really about 'left: auto' offset. So, in all fairness, you were destined
for a test filename change anyway.

2-
<!DOCTYPE html SYSTEM "about:legacy-compat">

Please use

<!DOCTYPE html>
<meta charset="utf-8">
etc..
just like in the test template:
http://testthewebforward.org/docs/test-templates.html#reftest-including-metadata

or use

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN"
"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">


3-
     <link rel="help"
href="https://drafts.csswg.org/css2/visuren.html#choose-position" />
     <link rel="help"
href="https://drafts.csswg.org/css2/visudet.html#abs-non-replaced-width"
/>
     <link rel="help"
href="https://drafts.csswg.org/css2/visudet.html#abs-non-replaced-height"
/>

Do not link to draft spec as they can change or will change in the
future. Shepherd also warns against this. And do not use too many links
to spec: here, the test would be inserted into 3 sections of the CSS2.1
(or 2.2) test suite... In my opinion, the last one
(visudet.html#abs-non-replaced-height) is not relevant to your test.

4-
     <meta name="assert" content="An element which is position:fixed
should have its position calculated correctly using the absolute model
when its top/bottom/left/right properties all use the default `auto`
value and its parent is position:relative." />

Avoid use of expressions like "should work correctly", "should be
implemented correctly", "should be supported correctly", "should be
calculated correctly" and expressions like those in the meta text assert
because they do not describe or explain in useful terms what the test is
supposed to be testing, checking.

5-
#shifted-column {
     left: 50%;
     width: 50%;
     position: relative;
     float: left;
}

Please explain what the float declaration is supposed to be doing in
that test. Is it really required by your test? Does it do something in
particular in your test? Is it a necessary and relevant component of
your test?
I believe the 'float: left' declaration should not be in your test at
all.

6-
#green {
   background-color: green;
   position: absolute;
   left: 50%;
   z-index: 2;
}

Since the green square is supposed to overlap the red square and since
it comes after the red square, then you do not need to set a z-index
since "Boxes with the same stack level in a stacking context are stacked
back-to-front according to document tree order."

7-
p {
   position: absolute;
   top: 100px;
}

It is recommended to start all tests with the pass-fail-conditions
sentence and, since it should not be part of the test itself, then we do
not want to style it in any way. Sometimes, this is not possible but,
I'd say 99% of the time, it is possible.



Here's what could be your test with all these changes:

http://www.gtalbot.org/BrowserBugsSection/css21testsuite/left-offset-position-fixed-001.xht

The 'left: auto' declaration was added on purpose, deliberately, even
though we know it is the default, initial box offset.

The reference file for that test has not been created yet.

One last thing. One other equivalent test could be created with
'direction: rtl', 'position fixed', 'right: auto' for many reasons.
Also, same 2 tests but with 'position: absolute'. So, 3 other additional
tests with the same basis code are possible here.

GĂ©rard
--
Test Format Guidelines
http://testthewebforward.org/docs/test-format-guidelines.html

Test Style Guidelines
http://testthewebforward.org/docs/test-style-guidelines.html

Test Templates
http://testthewebforward.org/docs/test-templates.html

CSS Naming Guidelines
http://testthewebforward.org/docs/css-naming.html

Test Review Checklist
http://testthewebforward.org/docs/review-checklist.html

CSS Metadata
http://testthewebforward.org/docs/css-metadata.html

Reply | Threaded
Open this post in threaded view
|

Re: [CSS21][CSS22] Seeking review of position:fixed edge case test

Chris Rebert
On Tue, Aug 4, 2015 at 9:06 PM, GĂ©rard Talbot
<[hidden email]> wrote:
> Le 2015-07-27 03:21, Chris Rebert a Ă©crit :
>>
>> Hi folks,
>>
>> I'm trying to get a reviewer for a CSS2 regression test:
>> https://github.com/w3c/csswg-test/pull/813
<snip>
> Your test is a good test. Indeed iPhone 6, Safari 7+ fail your test. Here's
> some review comments.

Thank you very much for your comments! I've revised the pull request
accordingly:
https://github.com/w3c/csswg-test/pull/813

> 1- There is already a test with the filename position-fixed-001
>
> http://test.csswg.org/suites/css2.1/nightly-unstable/html4/position-fixed-001.htm
>
> in the test suite. Fortunately for you, your test, in my opinion, is really
> about 'left: auto' offset. So, in all fairness, you were destined for a test
> filename change anyway.

A. This "testcase filename itself must be globally unique" requirement
doesn't seem to be documented anywhere on Test the Web Forward.
B. The "Automatic validation checks" performed by W3C Sync Bot
(https://github.com/w3c/csswg-test/pull/813#issuecomment-124963013 )
didn't complain about this. If it's really a requirement, this seems
like something that the bot could and should enforce.
C. Okay, I renamed it to "left-offset-position-fixed-001.xht"
following your example.

> 2-
> <!DOCTYPE html SYSTEM "about:legacy-compat">
>
> Please use
>
> <!DOCTYPE html>

Done.

> 3-
>     <link rel="help"
> href="https://drafts.csswg.org/css2/visuren.html#choose-position" />
>     <link rel="help"
> href="https://drafts.csswg.org/css2/visudet.html#abs-non-replaced-width" />
>     <link rel="help"
> href="https://drafts.csswg.org/css2/visudet.html#abs-non-replaced-height" />
>
> Do not link to draft spec as they can change or will change in the future.

Fixed.
Sorry, I had gotten confused since web-platform-tests uses the exact
opposite convention.

For the record, http://testthewebforward.org/docs/css-metadata.html
does not explicitly mention this rule, although its example does use
non-draft links.

Also, again, this seems like something that W3C Sync Bot ought to validate.

> Shepherd also warns against this. And do not use too many links to spec:
> here, the test would be inserted into 3 sections of the CSS2.1 (or 2.2) test
> suite... In my opinion, the last one (visudet.html#abs-non-replaced-height)
> is not relevant to your test.

Removed.
Yeah, I was grasping at straws a bit here for exactly what to cite.
Thanks for the guidance.

> 4-
>     <meta name="assert" content="An element which is position:fixed should
> have its position calculated correctly using the absolute model when its
> top/bottom/left/right properties all use the default `auto` value and its
> parent is position:relative." />
>
> Avoid use of expressions like "should work correctly",

Changed this to use the superior description from your version.

> 5-
> #shifted-column {
>     left: 50%;
>     width: 50%;
>     position: relative;
>     float: left;
> }
>
> Please explain what the float declaration is supposed to be doing in that
> test. Is it really required by your test? Does it do something in particular
> in your test? Is it a necessary and relevant component of your test?
> I believe the 'float: left' declaration should not be in your test at all.

Removed.
It was part of the original grid system CSS which the testcase was
derived from, but it turns out to be irrelevant/unnecessary for this
test.

> 6-
> #green {
>   background-color: green;
>   position: absolute;
>   left: 50%;
>   z-index: 2;
> }
>
> Since the green square is supposed to overlap the red square and since it
> comes after the red square, then you do not need to set a z-index since
> "Boxes with the same stack level in a stacking context are stacked
> back-to-front according to document tree order."

Removed. I was being paranoid.

> 7-
> p {
>   position: absolute;
>   top: 100px;
> }
>
> It is recommended to start all tests with the pass-fail-conditions sentence
> and, since it should not be part of the test itself, then we do not want to
> style it in any way. Sometimes, this is not possible but, I'd say 99% of the
> time, it is possible.

The position:fixed makes doing this somewhat tricky.
If you have a specific suggestion for how to achieve this, I'm fine
with changing the test, but I myself don't have any ideas.

> Here's what could be your test with all these changes:
>
> http://www.gtalbot.org/BrowserBugsSection/css21testsuite/left-offset-position-fixed-001.xht
>
> The 'left: auto' declaration was added on purpose, deliberately, even though
> we know it is the default, initial box offset.

Added this `left:auto`.

> The reference file for that test has not been created yet.
>
> One last thing. One other equivalent test could be created with 'direction:
> rtl', 'position fixed', 'right: auto' for many reasons. Also, same 2 tests
> but with 'position: absolute'. So, 3 other additional tests with the same
> basis code are possible here.

I'm not familiar enough with `direction`/RTL to write those variants,
but I'll be happy to write the `position:absolute` variant as soon as
the current testcase is approved.

Cheers,
Chris
--
Bootstrap Core Team member
Browser 🐛 of the day: http://wkbug.com/148061

Reply | Threaded
Open this post in threaded view
|

Re: [CSS21][CSS22] Seeking review of position:fixed edge case test

GĂ©rard Talbot-3
Le 2015-08-17 15:08, Chris Rebert a écrit :

> On Tue, Aug 4, 2015 at 9:06 PM, GĂ©rard Talbot
> <[hidden email]> wrote:
>> Le 2015-07-27 03:21, Chris Rebert a Ă©crit :
>>>
>>> Hi folks,
>>>
>>> I'm trying to get a reviewer for a CSS2 regression test:
>>> https://github.com/w3c/csswg-test/pull/813
> <snip>
>> Your test is a good test. Indeed iPhone 6, Safari 7+ fail your test.
>> Here's
>> some review comments.
>
> Thank you very much for your comments! I've revised the pull request
> accordingly:
> https://github.com/w3c/csswg-test/pull/813

I do not use github. I'll try to find your test anyway...

>
>> 1- There is already a test with the filename position-fixed-001
>>
>> http://test.csswg.org/suites/css2.1/nightly-unstable/html4/position-fixed-001.htm
>>
>> in the test suite. Fortunately for you, your test, in my opinion, is
>> really
>> about 'left: auto' offset. So, in all fairness, you were destined for
>> a test
>> filename change anyway.
>
> A. This "testcase filename itself must be globally unique" requirement
> doesn't seem to be documented anywhere on Test the Web Forward.

When the first version of the file format documentation was created,
zero-filled number suffix was explained to keep filename unique. As more
and more test suites for 40+ specifications were created, there is a
need to keep filename unique... otherwise test searching (with CSS Test
Suite Manager Shepherd system: http://test.csswg.org/shepherd/ ) gets
more difficult, confusing and returning several occurences.

But, you are right; I agree with you that this "testcase filename itself
must be globally unique" guideline or requirement should be stated
somewhere in the Test the Web Forward documentation.

> B. The "Automatic validation checks" performed by W3C Sync Bot
> (https://github.com/w3c/csswg-test/pull/813#issuecomment-124963013 )
> didn't complain about this. If it's really a requirement, this seems
> like something that the bot could and should enforce.
> C. Okay, I renamed it to "left-offset-position-fixed-001.xht"
> following your example.
>
>> 2-
>> <!DOCTYPE html SYSTEM "about:legacy-compat">
>>
>> Please use
>>
>> <!DOCTYPE html>
>
> Done.
>
>> 3-
>>     <link rel="help"
>> href="https://drafts.csswg.org/css2/visuren.html#choose-position" />
>>     <link rel="help"
>> href="https://drafts.csswg.org/css2/visudet.html#abs-non-replaced-width"
>> />
>>     <link rel="help"
>> href="https://drafts.csswg.org/css2/visudet.html#abs-non-replaced-height"
>> />
>>
>> Do not link to draft spec as they can change or will change in the
>> future.
>
> Fixed.
> Sorry, I had gotten confused since web-platform-tests uses the exact
> opposite convention.
>
> For the record, http://testthewebforward.org/docs/css-metadata.html
> does not explicitly mention this rule, although its example does use
> non-draft links.

Then, in my opinion, it should state such guideline and then be updated
accordingly.

Such guideline is not mentioned in the documentation. As a test author,
you want to provide a link to the most stable version of a spec so that
your test remains as much reliable and complete as possible. Once a spec
becomes Proposed Recommendation or Technical Recommendation, then *I
think (speculation)* working drafts no longer exist and links are not
redirected.

"
This is a draft document and may be updated, replaced or obsoleted by
other documents at any time. It is inappropriate to cite this document
as other than work in progress.
"
https://drafts.csswg.org/css2/#status


>
> Also, again, this seems like something that W3C Sync Bot ought to
> validate.
>

I do not use or know about that W3C Sync Bot; I think (just a
hunch/speculation) such software only synchronized between github and
Mercurial ... but I really don't know...

>> Shepherd also warns against this. And do not use too many links to
>> spec:
>> here, the test would be inserted into 3 sections of the CSS2.1 (or
>> 2.2) test
>> suite... In my opinion, the last one
>> (visudet.html#abs-non-replaced-height)
>> is not relevant to your test.
>
> Removed.
> Yeah, I was grasping at straws a bit here for exactly what to cite.
> Thanks for the guidance.
>
>> 4-
>>     <meta name="assert" content="An element which is position:fixed
>> should
>> have its position calculated correctly using the absolute model when
>> its
>> top/bottom/left/right properties all use the default `auto` value and
>> its
>> parent is position:relative." />
>>
>> Avoid use of expressions like "should work correctly",
>
> Changed this to use the superior description from your version.
>
>> 5-
>> #shifted-column {
>>     left: 50%;
>>     width: 50%;
>>     position: relative;
>>     float: left;
>> }
>>
>> Please explain what the float declaration is supposed to be doing in
>> that
>> test. Is it really required by your test? Does it do something in
>> particular
>> in your test? Is it a necessary and relevant component of your test?
>> I believe the 'float: left' declaration should not be in your test at
>> all.
>
> Removed.
> It was part of the original grid system CSS which the testcase was
> derived from, but it turns out to be irrelevant/unnecessary for this
> test.
>
>> 6-
>> #green {
>>   background-color: green;
>>   position: absolute;
>>   left: 50%;
>>   z-index: 2;
>> }
>>
>> Since the green square is supposed to overlap the red square and since
>> it
>> comes after the red square, then you do not need to set a z-index
>> since
>> "Boxes with the same stack level in a stacking context are stacked
>> back-to-front according to document tree order."
>
> Removed. I was being paranoid.
>
>> 7-
>> p {
>>   position: absolute;
>>   top: 100px;
>> }
>>
>> It is recommended to start all tests with the pass-fail-conditions
>> sentence
>> and, since it should not be part of the test itself, then we do not
>> want to
>> style it in any way. Sometimes, this is not possible but, I'd say 99%
>> of the
>> time, it is possible.
>
> The position:fixed makes doing this somewhat tricky.
> If you have a specific suggestion for how to achieve this, I'm fine
> with changing the test, but I myself don't have any ideas.

The modified test I created made that possible.

>
>> Here's what could be your test with all these changes:
>>
>> http://www.gtalbot.org/BrowserBugsSection/css21testsuite/left-offset-position-fixed-001.xht
>>
>> The 'left: auto' declaration was added on purpose, deliberately, even
>> though
>> we know it is the default, initial box offset.
>
> Added this `left:auto`.
>
>> The reference file for that test has not been created yet.
>>
>> One last thing. One other equivalent test could be created with
>> 'direction:
>> rtl', 'position fixed', 'right: auto' for many reasons. Also, same 2
>> tests
>> but with 'position: absolute'. So, 3 other additional tests with the
>> same
>> basis code are possible here.
>
> I'm not familiar enough with `direction`/RTL to write those variants,

http://www.gtalbot.org/BrowserBugsSection/css21testsuite/right-offset-position-fixed-001.xht

iPhone 6, Safari 7+ fail this additional test.
The text assert and a few other things in that
right-offset-position-fixed-001 would need to be adjusted, tweaked.

This new test proves that
http://test.csswg.org/suites/css2.1/nightly-unstable/html4/bidi-position-fixed-001.htm
was basic or was not very challenging.

> but I'll be happy to write the `position:absolute` variant as soon as
> the current testcase is approved.

I use Mercurial. If your test(s) is(are) not in

http://test.csswg.org/source/

or not in

http://test.csswg.org/source/css21/

then I can not approve them. Right now, I do not see your test anywhere
in Shepherd system and in test.csswg.org/source/ .
I speculate this is what W3C Sync Bot ... it should make submitted tests
in both systems ... but if there is no
http://test.csswg.org/source/css22/ then the test can not be made
available.

GĂ©rard

>
> Cheers,
> Chris
> --
> Bootstrap Core Team member
> Browser 🐛 of the day: http://wkbug.com/148061

--
Test Format Guidelines
http://testthewebforward.org/docs/test-format-guidelines.html

Test Style Guidelines
http://testthewebforward.org/docs/test-style-guidelines.html

Test Templates
http://testthewebforward.org/docs/test-templates.html

CSS Naming Guidelines
http://testthewebforward.org/docs/css-naming.html

Test Review Checklist
http://testthewebforward.org/docs/review-checklist.html

CSS Metadata
http://testthewebforward.org/docs/css-metadata.html

Reply | Threaded
Open this post in threaded view
|

Re: [CSS21][CSS22] Seeking review of position:fixed edge case test

Peter Linss

On Aug 17, 2015, at 1:49 PM, GĂ©rard Talbot <[hidden email]> wrote:

Le 2015-08-17 15:08, Chris Rebert a Ă©crit :
On Tue, Aug 4, 2015 at 9:06 PM, GĂ©rard Talbot
<[hidden email]> wrote:
Le 2015-07-27 03:21, Chris Rebert a Ă©crit :

1- There is already a test with the filename position-fixed-001
http://test.csswg.org/suites/css2.1/nightly-unstable/html4/position-fixed-001.htm
in the test suite. Fortunately for you, your test, in my opinion, is really
about 'left: auto' offset. So, in all fairness, you were destined for a test
filename change anyway.
A. This "testcase filename itself must be globally unique" requirement
doesn't seem to be documented anywhere on Test the Web Forward.

When the first version of the file format documentation was created, zero-filled number suffix was explained to keep filename unique. As more and more test suites for 40+ specifications were created, there is a need to keep filename unique... otherwise test searching (with CSS Test Suite Manager Shepherd system: http://test.csswg.org/shepherd/ ) gets more difficult, confusing and returning several occurences.

But, you are right; I agree with you that this "testcase filename itself must be globally unique" guideline or requirement should be stated somewhere in the Test the Web Forward documentation.

Agreed, this used to be stated but apparently has gotten lost as the docs have merged and transitioned.

We have this requirement because test sources get built into one or more test suites (depending on their spec links). The built test suites have a flat directory structure and do not attempt to preserve the source directory structure, so filename collisions can happen.


B. The "Automatic validation checks" performed by W3C Sync Bot
(https://github.com/w3c/csswg-test/pull/813#issuecomment-124963013 )
didn't complain about this. If it's really a requirement, this seems
like something that the bot could and should enforce.
C. Okay, I renamed it to "left-offset-position-fixed-001.xht"
following your example.
2-
<!DOCTYPE html SYSTEM "about:legacy-compat">
Please use
<!DOCTYPE html>
Done.
3-
   <link rel="help"
href="https://drafts.csswg.org/css2/visuren.html#choose-position" />
   <link rel="help"
href="https://drafts.csswg.org/css2/visudet.html#abs-non-replaced-width" />
   <link rel="help"
href="https://drafts.csswg.org/css2/visudet.html#abs-non-replaced-height" />
Do not link to draft spec as they can change or will change in the future.
Fixed.
Sorry, I had gotten confused since web-platform-tests uses the exact
opposite convention.
For the record, http://testthewebforward.org/docs/css-metadata.html
does not explicitly mention this rule, although its example does use
non-draft links.

Then, in my opinion, it should state such guideline and then be updated accordingly.

Such guideline is not mentioned in the documentation. As a test author, you want to provide a link to the most stable version of a spec so that your test remains as much reliable and complete as possible. Once a spec becomes Proposed Recommendation or Technical Recommendation, then *I think (speculation)* working drafts no longer exist and links are not redirected.

"
This is a draft document and may be updated, replaced or obsoleted by other documents at any time. It is inappropriate to cite this document as other than work in progress.
"
https://drafts.csswg.org/css2/#status


This is actually not a requirement. While it's best for test to link to the most stable version of a spec, linking to a draft is fine, and sometimes necessary if the tested features are not published on /TR yet. Our tools deal with draft links just fine. 

It is possible however for anchors to change in drafts, and if that happens the tests will beed to be updated, but as drafts mature we do try to avoid it.


Also, again, this seems like something that W3C Sync Bot ought to validate.

It does check the links, if it's not complaining about the drafts links, then they're fine.


I do not use or know about that W3C Sync Bot; I think (just a hunch/speculation) such software only synchronized between github and Mercurial ... but I really don't know...

SyncBot is an account on GutHub that Shepherd uses to sync the GitHub and Mercurial repositories. Shepherd also looks at submitted pull requests and runs the same validation checks that it does when files are pushed to the repo, these "pre-warnings" are what SyncBot generates on GitHub.



signature.asc (506 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [CSS21][CSS22] Seeking review of position:fixed edge case test

Chris Rebert
In reply to this post by GĂ©rard Talbot-3
On Mon, Aug 17, 2015 at 1:49 PM, GĂ©rard Talbot
<[hidden email]> wrote:
> Le 2015-08-17 15:08, Chris Rebert a Ă©crit :
>> On Tue, Aug 4, 2015 at 9:06 PM, GĂ©rard Talbot
>> <[hidden email]> wrote:
>>> Le 2015-07-27 03:21, Chris Rebert a Ă©crit :
<snip>

>>> 1- There is already a test with the filename position-fixed-001
>>>
>>> http://test.csswg.org/suites/css2.1/nightly-unstable/html4/position-fixed-001.htm
>>>
>>> in the test suite. Fortunately for you, your test, in my opinion, is really
>>> about 'left: auto' offset. So, in all fairness, you were destined for a test
>>> filename change anyway.
>>
>> A. This "testcase filename itself must be globally unique" requirement
>> doesn't seem to be documented anywhere on Test the Web Forward.
>
> When the first version of the file format documentation was created, zero-filled number suffix was explained to keep filename unique. As more and more test suites for 40+ specifications were created, there is a need to keep filename unique... otherwise test searching (with CSS Test Suite Manager Shepherd system: http://test.csswg.org/shepherd/ ) gets more difficult, confusing and returning several occurences.
>
> But, you are right; I agree with you that this "testcase filename itself must be globally unique" guideline or requirement should be stated somewhere in the Test the Web Forward documentation.

I went ahead earlier and filed a bug regarding this:
https://github.com/w3c/web-platform-tests/issues/2095

Cheers,
Chris
--
Bootstrap Core Team member
Browser 🐛 of the day: http://wkbug.com/148061

Reply | Threaded
Open this post in threaded view
|

Re: [CSS21][CSS22] Seeking review of position:fixed edge case test

GĂ©rard Talbot-3
In reply to this post by Peter Linss
Le 2015-08-17 19:10, Linss, Peter a écrit :
> On Aug 17, 2015, at 1:49 PM, GĂ©rard Talbot <[hidden email]>
> wrote:
>
>> Le 2015-08-17 15:08, Chris Rebert a Ă©crit :
>>> On Tue, Aug 4, 2015 at 9:06 PM, GĂ©rard Talbot
>>> <[hidden email]> wrote:
>>>> Le 2015-07-27 03:21, Chris Rebert a Ă©crit :

[snipped]

>>>> 3-
>>>>    <link rel="help"
>>>> href="https://drafts.csswg.org/css2/visuren.html#choose-position" />
>>>>    <link rel="help"
>>>> href="https://drafts.csswg.org/css2/visudet.html#abs-non-replaced-width"
>>>> />
>>>>    <link rel="help"
>>>> href="https://drafts.csswg.org/css2/visudet.html#abs-non-replaced-height"
>>>> />
>>>> Do not link to draft spec as they can change or will change in the
>>>> future.
>>> Fixed.
>>> Sorry, I had gotten confused since web-platform-tests uses the exact
>>> opposite convention.
>>> For the record, http://testthewebforward.org/docs/css-metadata.html
>>> does not explicitly mention this rule, although its example does use
>>> non-draft links.
>>
>> Then, in my opinion, it should state such guideline and then be
>> updated accordingly.
>>
>> Such guideline is not mentioned in the documentation. As a test
>> author, you want to provide a link to the most stable version of a
>> spec so that your test remains as much reliable and complete as
>> possible. Once a spec becomes Proposed Recommendation or Technical
>> Recommendation, then *I think (speculation)* working drafts no longer
>> exist and links are not redirected.
>>
>> "
>> This is a draft document and may be updated, replaced or obsoleted by
>> other documents at any time. It is inappropriate to cite this document
>> as other than work in progress.
>> "
>> https://drafts.csswg.org/css2/#status
>>
>
> This is actually not a requirement. While it's best for test to link
> to the most stable version of a spec, linking to a draft is fine, and
> sometimes necessary if the tested features are not published on /TR
> yet. Our tools deal with draft links just fine.

Peter, there is no css22 directory right now in
http://test.csswg.org/source/

If the test links to a section of css22, then where will that test be
inside
http://test.csswg.org/source/
? Just asking here

> It is possible however for anchors to change in drafts, and if that
> happens the tests will beed to be updated, but as drafts mature we do
> try to avoid it.

In my opinion, this is not likely. People will just use a link to a spec
(working draft, editor draft, CR, whatever here) and forget about that
test once submitted.

GĂ©rard
--
Test Format Guidelines
http://testthewebforward.org/docs/test-format-guidelines.html

Test Style Guidelines
http://testthewebforward.org/docs/test-style-guidelines.html

Test Templates
http://testthewebforward.org/docs/test-templates.html

CSS Naming Guidelines
http://testthewebforward.org/docs/css-naming.html

Test Review Checklist
http://testthewebforward.org/docs/review-checklist.html

CSS Metadata
http://testthewebforward.org/docs/css-metadata.html

Reply | Threaded
Open this post in threaded view
|

Re: [CSS21][CSS22] Seeking review of position:fixed edge case test

Chris Rebert
In reply to this post by GĂ©rard Talbot-3
On Mon, Aug 17, 2015 at 1:49 PM, GĂ©rard Talbot
<[hidden email]> wrote:

> Le 2015-08-17 15:08, Chris Rebert a Ă©crit :
>> On Tue, Aug 4, 2015 at 9:06 PM, GĂ©rard Talbot
>> <[hidden email]> wrote:
>>> Le 2015-07-27 03:21, Chris Rebert a Ă©crit :
>>>>
>>>> Hi folks,
>>>>
>>>> I'm trying to get a reviewer for a CSS2 regression test:
>>>> https://github.com/w3c/csswg-test/pull/813
>> <snip>
>>> Your test is a good test. Indeed iPhone 6, Safari 7+ fail your test. Here's
>>> some review comments.
<snip>

>>> 7-
>>> p {
>>>   position: absolute;
>>>   top: 100px;
>>> }
>>>
>>> It is recommended to start all tests with the pass-fail-conditions sentence
>>> and, since it should not be part of the test itself, then we do not want to
>>> style it in any way. Sometimes, this is not possible but, I'd say 99% of the
>>> time, it is possible.
>>
>> The position:fixed makes doing this somewhat tricky.
>> If you have a specific suggestion for how to achieve this, I'm fine
>> with changing the test, but I myself don't have any ideas.
>
> The modified test I created made that possible.

Revised.

>>> Here's what could be your test with all these changes:
>>>
>>> http://www.gtalbot.org/BrowserBugsSection/css21testsuite/left-offset-position-fixed-001.xht
<snip>

>>> One last thing. One other equivalent test could be created with 'direction:
>>> rtl', 'position fixed', 'right: auto' for many reasons. Also, same 2 tests
>>> but with 'position: absolute'. So, 3 other additional tests with the same
>>> basis code are possible here.
>>
>> I'm not familiar enough with `direction`/RTL to write those variants,
>
> http://www.gtalbot.org/BrowserBugsSection/css21testsuite/right-offset-position-fixed-001.xht
>
> iPhone 6, Safari 7+ fail this additional test.
> The text assert and a few other things in that right-offset-position-fixed-001 would need to be adjusted, tweaked.

Added the RTL and position:absolute tests you suggested.

Everything look good now?:
https://github.com/w3c/csswg-test/pull/813/files

Thanks,
Chris
--
Bootstrap Core Team member
Browser 🐛 of the day: http://bugzil.la/1203844

Reply | Threaded
Open this post in threaded view
|

Re: [CSS21][CSS22] Seeking review of position:fixed edge case test

GĂ©rard Talbot-3
Le 2015-09-13 00:30, Chris Rebert a écrit :

> On Mon, Aug 17, 2015 at 1:49 PM, GĂ©rard Talbot
> <[hidden email]> wrote:
>> Le 2015-08-17 15:08, Chris Rebert a Ă©crit :
>>> On Tue, Aug 4, 2015 at 9:06 PM, GĂ©rard Talbot
>>> <[hidden email]> wrote:
>>>> Le 2015-07-27 03:21, Chris Rebert a Ă©crit :
>>>>>
>>>>> Hi folks,
>>>>>
>>>>> I'm trying to get a reviewer for a CSS2 regression test:
>>>>> https://github.com/w3c/csswg-test/pull/813
>>> <snip>
>>>> Your test is a good test. Indeed iPhone 6, Safari 7+ fail your test.
>>>> Here's
>>>> some review comments.
> <snip>
>>>> 7-
>>>> p {
>>>>   position: absolute;
>>>>   top: 100px;
>>>> }
>>>>
>>>> It is recommended to start all tests with the pass-fail-conditions
>>>> sentence
>>>> and, since it should not be part of the test itself, then we do not
>>>> want to
>>>> style it in any way. Sometimes, this is not possible but, I'd say
>>>> 99% of the
>>>> time, it is possible.
>>>
>>> The position:fixed makes doing this somewhat tricky.
>>> If you have a specific suggestion for how to achieve this, I'm fine
>>> with changing the test, but I myself don't have any ideas.
>>
>> The modified test I created made that possible.
>
> Revised.
>
>>>> Here's what could be your test with all these changes:
>>>>
>>>> http://www.gtalbot.org/BrowserBugsSection/css21testsuite/left-offset-position-fixed-001.xht
> <snip>
>>>> One last thing. One other equivalent test could be created with
>>>> 'direction:
>>>> rtl', 'position fixed', 'right: auto' for many reasons. Also, same 2
>>>> tests
>>>> but with 'position: absolute'. So, 3 other additional tests with the
>>>> same
>>>> basis code are possible here.
>>>
>>> I'm not familiar enough with `direction`/RTL to write those variants,
>>
>> http://www.gtalbot.org/BrowserBugsSection/css21testsuite/right-offset-position-fixed-001.xht
>>
>> iPhone 6, Safari 7+ fail this additional test.
>> The text assert and a few other things in that
>> right-offset-position-fixed-001 would need to be adjusted, tweaked.
>
> Added the RTL and position:absolute tests you suggested.
>
> Everything look good now?:
> https://github.com/w3c/csswg-test/pull/813/files
>
> Thanks,
> Chris


Chris,

Sorry for the long delay.

I also had all kinds of difficulties with your tests, GitHub, filenames,
browsers, etc.

As strange as it may seem, it's possible to save a file under a filename
that has a white blank space (U+0020) appended at the end of the file
extension: this is news to me! In such case, Firefox 40.0.3 will load
such file but Chrome 45.0.2454.93 will not and will only say that the
file is not found "ERR_FILE_NOT_FOUND" but that's because the address
bar, it seems, trims out blank spaces. I think the text editor I used is
the software with a bug: it should trim out the blank space at the end.

1-
With GitHub, I have to find the files, then I have to copy and paste the
code into a text editor, then remove the "+" [addendum: I just noticed
the View button for viewing the whole file without "+"], then save the
code under suitable filename(s) and then load the tests in browsers for
examination. There seems no other way to review tests with GitHub.

2-
The main difference between absolute positioning and fixed positioning
is what's a box's containing block. Section 10.1 states:

"
If the element has 'position: fixed', the containing block is
established by the viewport (...)

If the element has 'position: absolute', the containing block is
established by the nearest ancestor with a 'position' of 'absolute',
'relative' or 'fixed', (...).
"
10.1 Definition of "containing block"
http://www.w3.org/TR/CSS21/visudet.html#containing-block-details

About abs. positioning, we often use expressions like nearest (or
closest) positioned container block within containment hierarchy or
nearest (or closest) positioned ancestor.

So, your text asserts need to reflect such absolute-versus-fixed
difference and they are not.

- - - - - - - -

The 'left: auto' and 'right: auto' is given by (or, if you prefer, is
established by) the so-called "static-position containing block". Often,
this is the immediate (nearest, closest) parent of such box. In your
tests, the "static-position containing block" of your div#red is your
div#shifted-column .

- - - - - - - -

Your href="http://chrisrebert.com" leads to 404 not found ... but that's
a minor thing.

- - - - - - - -

Chris, for many reasons, I propose you just submit your position-fixed
tests (along with the reference files) and leave out, drop the abs-pos
tests. I approve your
left-offset-position-fixed-001.xht
and
right-offset-position-fixed-001.xht
along with their respective reference files. Please add the date like
this:

     <link rel="reviewer" title="GĂ©rard Talbot"
href="http://www.gtalbot.org/BrowserBugsSection/css21testsuite/" /> <!--
2015-09-17 -->


GĂ©rard
--
Test Format Guidelines
http://testthewebforward.org/docs/test-format-guidelines.html

Test Style Guidelines
http://testthewebforward.org/docs/test-style-guidelines.html

Test Templates
http://testthewebforward.org/docs/test-templates.html

CSS Naming Guidelines
http://testthewebforward.org/docs/css-naming.html

Test Review Checklist
http://testthewebforward.org/docs/review-checklist.html

CSS Metadata
http://testthewebforward.org/docs/css-metadata.html

Reply | Threaded
Open this post in threaded view
|

Re: [CSS21][CSS22] Seeking review of position:fixed edge case test

Chris Rebert
On Wed, Sep 16, 2015 at 10:11 PM, GĂ©rard Talbot
<[hidden email]> wrote:
<snip>
>> Everything look good now?:
>> https://github.com/w3c/csswg-test/pull/813/files
>
> Chris,
>
> Sorry for the long delay.
>
> I also had all kinds of difficulties with your tests, GitHub, filenames,
> browsers, etc.

No worries. Sorry to hear you had a rough time with the logistics.

<snip>
> 1-
> With GitHub, I have to find the files, then I have to copy and paste the
> code into a text editor, then remove the "+" [addendum: I just noticed the
> View button for viewing the whole file without "+"], then save the code
> under suitable filename(s) and then load the tests in browsers for
> examination. There seems no other way to review tests with GitHub.

For future reference, after clicking the "View" button, the next page
has a "Raw" button which shows the plain text of the file.

<snip>
> - - - - - - - -
>
> Your href="http://chrisrebert.com" leads to 404 not found ... but that's a
> minor thing.

Yeah, setting up my new blog is still on my to-do list. It's headed
towards the top of the list though.

> - - - - - - - -
>
> Chris, for many reasons, I propose you just submit your position-fixed tests
> (along with the reference files) and leave out, drop the abs-pos tests. I
> approve your
> left-offset-position-fixed-001.xht
> and
> right-offset-position-fixed-001.xht
> along with their respective reference files. Please add the date like this:
>
>     <link rel="reviewer" title="GĂ©rard Talbot"
> href="http://www.gtalbot.org/BrowserBugsSection/css21testsuite/" /> <!--
> 2015-09-17 -->

Done. Thank you very much for the approvals!
Now I'll try asking one of the CSSWG members who is on GitHub to merge
the tests on the basis of your approvals.

Cheers,
Chris
--
Bootstrap Core Team Member
Browser 🐛 of the day: http://wkbug.com/146244