Bug 202148 - right- and bottom-relative values in background-position-x/y don't work
Summary: right- and bottom-relative values in background-position-x/y don't work
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: Safari 12
Hardware: Mac macOS 10.14
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: BrowserCompat, InRadar, WPTImpact
Depends on:
Blocks:
 
Reported: 2019-09-24 09:48 PDT by John A. Bilicki III
Modified: 2021-09-09 12:14 PDT (History)
11 users (show)

See Also:


Attachments
Potrait mode screenshot from an iPhone 6S. (127.05 KB, image/png)
2019-09-25 15:49 PDT, John A. Bilicki III
no flags Details
Gecko and WebKit background-position-x comparison PNG image. (17.46 KB, image/png)
2021-09-08 09:27 PDT, John A. Bilicki III
no flags Details
Simple testcase (557 bytes, text/html)
2021-09-08 10:23 PDT, Simon Fraser (smfr)
no flags Details
Patch (22.47 KB, patch)
2021-09-08 13:02 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (23.33 KB, patch)
2021-09-08 14:08 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (28.85 KB, patch)
2021-09-08 16:01 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (31.12 KB, patch)
2021-09-08 16:32 PDT, Simon Fraser (smfr)
koivisto: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John A. Bilicki III 2019-09-24 09:48:31 PDT
The CSS:
[required] {background-position: top right;}

On an example URL (any blog entry page with h2 > input for the comment header):
https://www.jabcreations.com/blog/json-encoding-of-multi-dimensional-javascript-array

With the element that can be selected in the DOM via:
document.getElementById('post_title')

Incorrectly renders the SVG image at the top-left instead of the top-right in Safari 12.1.2.
Comment 1 Alexey Proskuryakov 2019-09-25 15:33:23 PDT
This page seems to look the same for me between Safari 13 and Chrome. Could you please attach screenshots of expected and actual rendering?
Comment 2 John A. Bilicki III 2019-09-25 15:49:41 PDT
Created attachment 379586 [details]
Potrait mode screenshot from an iPhone 6S.

I should have clarified this was Safari using an iPhone 6S. This occurs in both portrait and landscape orientations and I'll fix the unrelated rendering errors.
Comment 3 Simon Fraser (smfr) 2019-10-23 15:53:49 PDT
The rule is:
background-position-x: right calc(var(--size_layout_border_radius) / 2) !important;
which is resolving to 0%. This is more likely to be about calc than background-position.
Comment 4 Simon Fraser (smfr) 2021-08-26 16:12:33 PDT
It's really hard to tell because the page is a mess, and it looks like Chrome has the same rendering.

Please attach a more obvious testcase.
Comment 5 John A. Bilicki III 2021-09-07 00:50:51 PDT
Apparently WebKit doesn't support the two value syntax:

https://developer.mozilla.org/en-US/docs/Web/CSS/background-position-x#browser_compatibility

"It's really hard to tell because the page is a mess"

I presume since your email address is @webkit.org that you're inherently qualified as an engineer who speaks with other engineers. So let's try this one more time:

Copy - the following code:

document.getElementById('post_title')

Paste it - in to the developer console. It'll highlight the element. If you have a small screen you can scroll down.
Comment 6 Simon Fraser (smfr) 2021-09-07 09:19:29 PDT
I was able to easily find the element. I was not able to see how the rendering was incorrect.
Comment 7 John A. Bilicki III 2021-09-08 09:27:43 PDT
Created attachment 437636 [details]
Gecko and WebKit background-position-x comparison PNG image.
Comment 8 John A. Bilicki III 2021-09-08 09:29:25 PDT
I should mention that the second value is used because the input element has a border-radius so I need to use the second value offset to move it away from the right-most edge. I could use a high value from the left however the rendering would be drastic between tiny mobile screens and large displays of which my visitors use both and everything in between.
Comment 9 Simon Fraser (smfr) 2021-09-08 10:23:00 PDT
Clarified by screeshot
Comment 10 Simon Fraser (smfr) 2021-09-08 10:23:20 PDT
Created attachment 437646 [details]
Simple testcase
Comment 11 Simon Fraser (smfr) 2021-09-08 10:37:26 PDT
It seems that what doesn't work is specifically the right-relative form.
Comment 12 Simon Fraser (smfr) 2021-09-08 10:40:49 PDT
Seems like we don't support `background-position-x: right 30%;` without calc (nor does Chrome).
Comment 13 Simon Fraser (smfr) 2021-09-08 13:02:53 PDT
Created attachment 437658 [details]
Patch
Comment 14 Simon Fraser (smfr) 2021-09-08 14:08:20 PDT
Created attachment 437663 [details]
Patch
Comment 15 Darin Adler 2021-09-08 15:39:06 PDT
Comment on attachment 437663 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=437663&action=review

> Source/WebCore/css/Pair.h:43
> +        ASSERT(first);
> +        ASSERT(second);

Why would we do this? If we can’t take nulls, then why not have the arguments be Ref<>?
Comment 16 Simon Fraser (smfr) 2021-09-08 15:45:46 PDT
(In reply to Darin Adler from comment #15)
> Comment on attachment 437663 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=437663&action=review
> 
> > Source/WebCore/css/Pair.h:43
> > +        ASSERT(first);
> > +        ASSERT(second);
> 
> Why would we do this? If we can’t take nulls, then why not have the
> arguments be Ref<>?

I was trying to catch a null in this pair early, and seeing if EWS hit the assertions. If not, I can make them refs.
Comment 17 Simon Fraser (smfr) 2021-09-08 16:01:28 PDT
Created attachment 437673 [details]
Patch
Comment 18 Simon Fraser (smfr) 2021-09-08 16:32:51 PDT
Created attachment 437676 [details]
Patch
Comment 19 Antti Koivisto 2021-09-08 23:00:52 PDT
Comment on attachment 437676 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=437676&action=review

> Source/WebCore/css/parser/CSSPropertyParserHelpers.h:49
> +enum class BoxOrient : uint8_t;

BoxEastAsia
Comment 20 Darin Adler 2021-09-09 10:06:37 PDT
Comment on attachment 437676 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=437676&action=review

> Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:2824
> +    if (value1 && !value2)
> +        return value1;
> +
> +    if (!value1 && value2)
> +        return value2;
> +
> +    return nullptr;

This can be written more simply like this:

    return value1 ? value1 : value2;

> Source/WebCore/css/parser/CSSPropertyParserHelpers.h:131
> +RefPtr<CSSPrimitiveValue> consumeSingleAxisPosition(CSSParserTokenRange&, CSSParserMode, BoxOrient, UnitlessQuirk = UnitlessQuirk::Forbid);

Do we need the UnitlessQuirk argument? Are there future clients who will want to pass it?
Comment 21 Simon Fraser (smfr) 2021-09-09 12:13:08 PDT
https://trac.webkit.org/changeset/282234/webkit
Comment 22 Radar WebKit Bug Importer 2021-09-09 12:14:19 PDT
<rdar://problem/82937796>