right- and bottom-relative values in background-position-x/y don't work

• Component: New Bugs
• Status: RESOLVED FIXED

[John A. Bilicki III]

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.

[Alexey Proskuryakov]

This page seems to look the same for me between Safari 13 and Chrome. Could you please attach screenshots of expected and actual rendering?

[John A. Bilicki III]

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.

[Simon Fraser (smfr)]

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.

[Simon Fraser (smfr)]

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.

[John A. Bilicki III]

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.

[Simon Fraser (smfr)]

I was able to easily find the element. I was not able to see how the rendering was incorrect.

[John A. Bilicki III]

Created attachment 437636 [details]
Gecko and WebKit background-position-x comparison PNG image.

[John A. Bilicki III]

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.

[Simon Fraser (smfr)]

Clarified by screeshot

[Simon Fraser (smfr)]

Created attachment 437646 [details]
Simple testcase

[Simon Fraser (smfr)]

It seems that what doesn't work is specifically the right-relative form.

[Simon Fraser (smfr)]

Seems like we don't support `background-position-x: right 30%;` without calc (nor does Chrome).

[Simon Fraser (smfr)]

Created attachment 437658 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 437663 [details]
Patch

[Darin Adler]

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<>?

[Simon Fraser (smfr)]

(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.

[Simon Fraser (smfr)]

Created attachment 437673 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 437676 [details]
Patch

[Antti Koivisto]

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

[Darin Adler]

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?

[Simon Fraser (smfr)]

https://trac.webkit.org/changeset/282234/webkit

[Radar WebKit Bug Importer]

<rdar://problem/82937796>

[Tim Nguyen (:ntim)]

*** Bug 157511 has been marked as a duplicate of this bug. ***