w3c / fxtf-drafts

Mirror of https://hg.fxtf.org/drafts
https://drafts.fxtf.org/
Other
68 stars 49 forks source link

[motion-1] Is `normal` the initial value for `offset-position`? #522

Closed nt1m closed 10 months ago

nt1m commented 1 year ago

"normal" usually refers to the default behavior usually, but offset-position's default value is auto, so this feels a bit odd.

Is there some potentially renaming to consider here?

@nmoucht @smfr

nt1m commented 1 year ago

@tabatkins Looks like this normal was meant to become the new default value from reading #504, can you check if the default value needs fixing?

nt1m commented 1 year ago

(I'd be OK closing out this issue with the default value changed to normal or having normal renamed to none if it's not the default)

cc @BorisChiou @danielsakhapov

danielsakhapov commented 1 year ago

I don't really have an opinion here, but we (Chrome and Firefox) already added tests and implementation for auto to be default and have a normal do what it is said to do.

nt1m commented 1 year ago

Quoting the IRC logs for the resolution:

fantasai: first comment if none is supposed to mean the default, then maybe it should be called normal instead

This is the reason normal was picked over none. Except the default never changed here.

tabatkins commented 11 months ago

Ah whoops you're right, my edit in https://github.com/w3c/fxtf-drafts/commit/43b2e46b9c16e160938c11e261c3a37700e2f404 forgot to change the initial value.

tabatkins commented 11 months ago

Keeping open because we'll need to tweak tests that expect the default value to be auto.

nt1m commented 11 months ago

I've updated the basic WPT testing the default value at: https://github.com/web-platform-tests/wpt/commit/b4ffc4e862ed65faba928940eb36d2e6d6806f73

@danielsakhapov @BorisChiou Would one of you be willing to make this change in Gecko / Blink and update relevant tests? I suspect some tests will need offset-position: auto to be explicitly specified.

BorisChiou commented 11 months ago

Filed Gecko bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1846817. Probably start to check it soon this week.

BorisChiou commented 11 months ago

I uploaded a Gecko patch (https://phabricator.services.mozilla.com/D185230). I will try to land it soon after it gets approval.