Status

()

defect
RESOLVED FIXED
7 months ago
5 months ago

People

(Reporter: emilio, Assigned: emilio)

Tracking

({dev-doc-complete, site-compat})

unspecified
mozilla64
Points:
---

Firefox Tracking Flags

(firefox63 fixed, firefox64 fixed)

Details

Attachments

(1 attachment)

Comment hidden (empty)
(Assignee)

Comment 1

7 months ago
See bug 1481866 Comment 14 as for why.
(Assignee)

Comment 2

7 months ago
The behavior the WG proposed is way more subtle than what that bug implements,
including:

 * Implementing two logical overflow longhands.
 * Expanding the overflow shorthand to different longhands depending on the
   syntax of that.

Meanwhile, Blink hasn't done the swap and will ship the same behavior that we
shipped in Firefox 61 (bug 1453148), that is, overflow-x, then overflow-y.

So I think lacking a clear way forward we should revert this change and preserve
our shipped behavior.
(Assignee)

Comment 3

7 months ago
Really sorry to have to revert the documentation changes and such :(
(Assignee)

Comment 4

7 months ago
Comment on attachment 9010418 [details]
Bug 1492567 - Back out bug 1481866. r=dbaron

Preemptively setting the approval flag before I forget.

Approval Request Comment
[Feature/Bug causing the regression]: Not really a regression, but bug 1481866.
[User impact if declined]: Behavior change that we probably don't want to ship as-is.
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: no, but it's a clean revert.
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: not risky
[Why is the change risky/not risky?]: This patch reverts a behavior change which I think we shouldn't change, per the  bug 1481866 comment 14. This reverts the behavior that we had since firefox 61, which is when this feature was implemented.
[String changes made/needed]: none
Attachment #9010418 - Flags: approval-mozilla-beta?
(Assignee)

Comment 5

7 months ago
(In reply to Emilio Cobos Álvarez (:emilio) from comment #4)
> [Why is the change risky/not risky?]: This patch reverts a behavior change
> which I think we shouldn't change, per the  bug 1481866 comment 14. This
> reverts the behavior that we had since firefox 61, which is when this
> feature was implemented.

per the *reasons described in* that comment, and reverts *to* the behavior we had since 61. Sorry, can't type.
Comment on attachment 9010418 [details]
Bug 1492567 - Back out bug 1481866. r=dbaron

David Baron :dbaron: 🏴󠁵󠁳󠁣󠁡󠁿 ⌚UTC-7 has approved the revision.
Attachment #9010418 - Flags: review+
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/13088 for changes under testing/web-platform/tests
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 10

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/cf9cd0662126
Status: NEW → RESOLVED
Last Resolved: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Upstream PR merged
Can't merge web-platform-tests PR due to failing upstream checks:
Github PR https://github.com/web-platform-tests/wpt/pull/13088
* Taskcluster (pull_request) (https://tools.taskcluster.net/task-group-inspector/#/PxeSFhjxRzCNzEVhLVDIhg)

Comment 13

7 months ago
I would understand if reverting to only accepting one value, but reverting to the opposite of the spec says means that we will end up with Web content depending on what you are shipping. I don't think this is a good plan. Either ship something compatible with the spec, or tell the CSSWG that you disagree with the spec and insist on it matching your implementation. Having the implementation intentionally do the opposite of what the spec says is not helpful to the Web.
Comment on attachment 9010418 [details]
Bug 1492567 - Back out bug 1481866. r=dbaron

Uplift approved for 63 beta 8, thanks.
Attachment #9010418 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(Assignee)

Comment 15

7 months ago
(In reply to fantasai from comment #13)
> I would understand if reverting to only accepting one value, but reverting
> to the opposite of the spec says means that we will end up with Web content
> depending on what you are shipping. I don't think this is a good plan.
> Either ship something compatible with the spec, or tell the CSSWG that you
> disagree with the spec and insist on it matching your implementation. Having
> the implementation intentionally do the opposite of what the spec says is
> not helpful to the Web.

I'm backing out to what we and other engines are shipping, to avoid shipping something that contradicts both the spec and our previous behavior. I wouldn't be confident to back this out to pre-61 behavior and getting that uplifted.

If you want to put the shorthand back to one value I can do that, but I don't think I should get that uplifted for 63. That being said if we go that rout we should ask the Blink folks to unship their shorthand as well.
Setting to DDC — we already reverted this in the docs.
You need to log in before you can comment on or make changes to this bug.