Bug 256814
Summary: | REGRESSION (Safari 16.4, 258767@main): Carcassonne game on boardgamearena.com unplayable (serialization bug affecting background-position) | ||
---|---|---|---|
Product: | WebKit | Reporter: | Andrew Collier <webkitbugzilla> |
Component: | DOM | Assignee: | Darin Adler <darin> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | ap, darin, koivisto, ntim, simon.fraser, webkit-bug-importer |
Priority: | P2 | Keywords: | InRadar |
Version: | Safari 16 | ||
Hardware: | All | ||
OS: | All | ||
See Also: | https://github.com/web-platform-tests/wpt/pull/40482 | ||
Bug Depends on: | |||
Bug Blocks: | 248913 |
Andrew Collier
This behaviour changed with the introduction of Safari 16.4. It is still present in Safari technology Preview Release 169 (Safari 16.4, WebKit 18616.1.12.2)
Given the following HTML:
<div id="outer" style="background-position: 10% 0%"></div>
Note the missing semicolon at the end of the style definition.
In Safari 16.4, this node's style.cssText becomes "background-position: 10%;"
In older Safari or any other browser, this node's style.cssText is "background-position: 10% 0%;"
Visible reproducer at https://jsfiddle.net/shadowphiar/skh5q6yz/
The reason this matters is that if any other style gets applied (such as a rotation transform) and background-position is recalculated, then suddenly the background-position-y is treated as 50% (because now only one argument is present in background-position). The writer expects background-position-y to remain 0%. I understand that the markup is incorrect but treating it so differently will be very unexpected.
This currently affects a production site boardgamearea.com and in particular their implementation of the game Carcassonne.
Attachments | ||
---|---|---|
Add attachment proposed patch, testcase, etc. |
Alexey Proskuryakov
Thank you for the report! Could you please add more details on how the website is affected?
I tried going there, but "boardgamearea.com" just redirected to some malicious content at first, and then at a placeholder after a minute or so.
Radar WebKit Bug Importer
<rdar://problem/109380755>
Alexey Proskuryakov
I don't have an older Safari readily available to confirm, but I suspect that serialization changed in bug 247879, while parsing may have always been wrong, defaulting to "0" by accident?
Andrew Collier
(In reply to Alexey Proskuryakov from comment #1)
> Thank you for the report! Could you please add more details on how the
> website is affected?
>
> I tried going there, but "boardgamearea.com" just redirected to some
> malicious content at first, and then at a placeholder after a minute or so.
Oh no! Sorry, that is a typo. I meant boardgamearena.com (with the n)
boardgamearena.com is quite a dynamic site, so page urls might not last long. Nonetheless here are a couple of examples currently visible live:
https://boardgamearena.com/3/carcassonne?table=377083053
https://boardgamearena.com/6/carcassonne?table=377145832
and there are screenshots of the issue in some forum threads:
https://boardgamearena.com/forum/viewtopic.php?t=29913
https://en.boardgamearena.com/bug?id=87361
https://imgur.com/ag3Sl21
I believe these should all be visible without needing to log in.
If you're unfamiliar with the game it may be difficult to see what the problem is, but notice that some of the tiles have a line exactly half way through, with building on one side and grass on the other, or roads the stop in the middle of the tile. This never happens on correctly-displayed tiles.
Alexey Proskuryakov
Thank you, this clearly makes the game unplayable.
Tim Nguyen (:ntim)
We use the initial value to choose whether to omit the longhand: https://searchfox.org/wubkat/rev/a33f8a24c752c23225ebeb863f4523b5f2ee4201/Source/WebCore/css/parser/CSSPropertyParser.cpp#1092-1096
While the initial value is indeed 0%, the serialization rule for `background-position` is different, if only one value is set, then 50% is used as Y coordinate. So serializing `10% 0%` as `10%` is clearly incorrect.
I think this is a regression of 259185@main.
Tim Nguyen (:ntim)
Actually, I think it's 258767@main
Darin Adler
So it’s not about the semicolon, it’s just a bug I introduced. Should be very easy to fix.
Darin Adler
Annoying that WPT did not catch this. I’ll have to add some WPT test cases too.
Darin Adler
Pull request: https://github.com/WebKit/WebKit/pull/14842
EWS
Committed 265056@main (82228ed93559): <https://commits.webkit.org/265056@main>
Reviewed commits have been landed. Closing PR #14842 and removing active labels.