Firefox already supports offset-block-start, offset-block-end, offset-inline-start and offset-inline-end. But the "offset-" prefix has been renamed to "inset-": https://github.com/w3c/csswg-drafts/issues/1519 Spec: https://drafts.csswg.org/css-logical-1/#inset-properties

Given that this is just a renaming, I think this should be somewhat higher priority since we don't want the web to get stuck depending on outdated names.

I agree. I guess we need to land an alias at least to leave activity-stream working until they update. David, do you think we should try to remove the alias shortly after this? (I don't think sites depend on this right now since we're the only implementer, but I don't have data on this...).

Maybe it makes sense to put the alias behind a pref and flip that pref off as soon as possible, but ship a release with the pref in the code (but off) before actually removing the code? (It's easier to ship a pref-flip if we need to than to ship a new release.)

Comment on attachment 8987936 [details] Bug 1464782: Rename offset-* properties to inset-*. https://reviewboard.mozilla.org/r/253218/#review259876 ::: servo/components/style/properties/longhands/position.mako.rs:32 (Diff revision 1) > - "offset-%s" % side, > + "inset-%s" % side, > "LengthOrPercentageOrAuto", > "computed::LengthOrPercentageOrAuto::Auto", > - spec="https://drafts.csswg.org/css-logical-props/#propdef-offset-%s" % side, > + spec="https://drafts.csswg.org/css-logical-props/#propdef-inset-%s" % side, > flags="GETCS_NEEDS_LAYOUT_FLUSH", > + alias="offset-%s" % side, I agree with dbaron that we should probably put it behind a pref which would make it easier to control. You can see https://searchfox.org/mozilla-central/rev/14cb8f1c238735ba1abe18ad44e39808983c2572/servo/components/style/properties/longhands/box.mako.rs#630 as an example for how to put an alias behind a pref.

Comment on attachment 8987938 [details] Bug 1464782: Update references to the properties in layout/. https://reviewboard.mozilla.org/r/253222/#review259880

Comment on attachment 8987939 [details] Bug 1464782: Some manual fixups to be squashed above. https://reviewboard.mozilla.org/r/253224/#review259882 ::: layout/style/test/property_database.js:6198 (Diff revision 1) > - } else if (/^offset-(block|inline)-(start|end)/.test(property)) { > - property = property.substring(7); // we want "top" not "offset-top", e.g. > + } else if (/^(offset|inset)-(block|inline)-(start|end)/.test(property)) { > + property = property.replace("offset-", "").replace("inset-", ""); // we want "top" not "offset-top", e.g. Why do we still have `offset-` here? Shouldn't they all have been replaced with `inset-` in this database? ::: layout/style/test/test_logical_properties.html:97 (Diff revision 1) > > if ((type == CSS_TYPE_SHORTHAND_AND_LONGHAND || > type == CSS_TYPE_LONGHAND && gCSSProperties[p].logical) && > /-inline-end/.test(p)) { > var valueType; > - if (/margin|padding|width|offset/.test(p)) { > + if (/margin|padding|width|inset|offset/.test(p)) { Again, why do we still need offset? ::: layout/style/test/test_logical_properties.html:113 (Diff revision 1) > inlineEnd: p, > blockStart: p.replace("-inline-end", "-block-start"), > blockEnd: p.replace("-inline-end", "-block-end"), > type: valueType > }; > - if (/^offset/.test(p)) { > + if (/^offset|inset/.test(p)) { In addition to the general offset problem, this regex doesn't work as it intended to do. `/^offset|inset/` means `/(^offset)|inset/` which means it matches `offset*` and `*inset*`, while it should probably match only `inset*`. That should be `/^(?:offset|inset)/` instead. But still, I wonder why we still need to test `offset` here. If you remove that, you wouldn't have any problem here.

Comment on attachment 8987940 [details] Bug 1464782: Test the aliases, for now. https://reviewboard.mozilla.org/r/253226/#review259884 ::: layout/style/test/property_database.js:6040 (Diff revision 1) > "calc(25px*3)", > "calc(3*25px + 50%)", > ], > invalid_values: [] > }, > + "offset-block-start": { Oh I guess this is the reason you need to deal with `offset-` in the previous patch? Makes sense. We can probably put it behind a pref as suggested before.

Comment on attachment 8987939 [details] Bug 1464782: Some manual fixups to be squashed above. https://reviewboard.mozilla.org/r/253224/#review259882 > In addition to the general offset problem, this regex doesn't work as it intended to do. > > `/^offset|inset/` means `/(^offset)|inset/` which means it matches `offset*` and `*inset*`, while it should probably match only `inset*`. That should be `/^(?:offset|inset)/` instead. > > But still, I wonder why we still need to test `offset` here. If you remove that, you wouldn't have any problem here. I guess the next patch should really be put before this patch so that some of the changes here makes more sense. Anyway, this regex is still a problem.

Comment on attachment 8987939 [details] Bug 1464782: Some manual fixups to be squashed above. https://reviewboard.mozilla.org/r/253224/#review259888

Comment on attachment 8987941 [details] Bug 1464782: Update references in the rest of the tree. https://reviewboard.mozilla.org/r/253228/#review259890

Comment on attachment 8987937 [details] Bug 1464782: Update a WPT test which is using the offset properties. https://reviewboard.mozilla.org/r/253220/#review259878

Comment on attachment 8988102 [details] Bug 1464782: Put it behind a pref. https://reviewboard.mozilla.org/r/253356/#review259976

Pushed by emilio@crisal.io: https://hg.mozilla.org/integration/mozilla-inbound/rev/7be80c866e84 Rename offset-* logical properties to inset-*. r=xidorn https://hg.mozilla.org/integration/mozilla-inbound/rev/f65ca6e2fb79 Update references to offset-* properties in the rest of the tree. r=xidorn https://hg.mozilla.org/integration/mozilla-inbound/rev/ecffedeea0e2 Put offset-* aliases behind a pref. r=xidorn

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/11689 for changes under testing/web-platform/tests

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Pushed by emilio@crisal.io: https://hg.mozilla.org/integration/mozilla-inbound/rev/d0b83e0056ac Allow logical aliases in the property database. r=me

Pushed by emilio@crisal.io: https://hg.mozilla.org/integration/mozilla-inbound/rev/5c07dcfb6368 Regenerate the devtools db. r=me

https://hg.mozilla.org/mozilla-central/rev/7be80c866e84 https://hg.mozilla.org/mozilla-central/rev/f65ca6e2fb79 https://hg.mozilla.org/mozilla-central/rev/ecffedeea0e2 https://hg.mozilla.org/mozilla-central/rev/d0b83e0056ac https://hg.mozilla.org/mozilla-central/rev/5c07dcfb6368

Oh, and there should be an intent to ship for this I believe (with an intent to unship for the old names).

Upstream PR merged

Sent one for the new properties and another for the deprecation in bug 1471838.

Commits pushed to master at https://github.com/mozilla/activity-stream https://github.com/mozilla/activity-stream/commit/936d51a0811aa5c8923e6bdfbb686fe7d68755da Backport changes from bug 1464782 https://github.com/mozilla/activity-stream/commit/f38c3275a06a8d898f727c2b8cd161bcc448182e Merge pull request #4217 from piatra/backport1464782 Backport changes from bug 1464782

Posted the site compatibility note: https://www.fxsitecompat.com/en-CA/docs/2018/offset-logical-properties-have-been-renamed-to-inset/

I've updated the docs for these properties, they can be found at: https://developer.mozilla.org/en-US/docs/inset-block-start https://developer.mozilla.org/en-US/docs/inset-block-end https://developer.mozilla.org/en-US/docs/inset-inline-start https://developer.mozilla.org/en-US/docs/inset-inline-end Compat data has also been updated. A review is appreciated - thank you!

Thanks! I haven't had the time to look super in-depth into all of them, but this is the only thing I found on the ones I did look up: The animation type says 'discrete', but that is no longer true since bug 1309752 / https://github.com/w3c/csswg-drafts/issues/2751. Other than that it looks amazing, thanks for all your work Rachel :)