WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
206909
REGRESSION (
r254406
): Gmail.com star/favorite icons are not rendering
https://bugs.webkit.org/show_bug.cgi?id=206909
Summary
REGRESSION (r254406): Gmail.com star/favorite icons are not rendering
Simon Fraser (smfr)
Reported
2020-01-28 14:44:28 PST
r254406
broke the icons in the "star" column in Gmail. The ::before has: content: -webkit-image-set('' 1x,'' 2x); It seems this was previously invalid, so that content:'' was used. But now it's treated as valid, so we show the missing image icon. I think we need to preserve the old behavior of image-set in -webkit-image-set.
Attachments
Patch
(10.93 KB, patch)
2020-01-29 00:52 PST
,
Noam Rosenthal
no flags
Details
Formatted Diff
Diff
Patch for landing
(10.89 KB, patch)
2020-01-30 04:27 PST
,
Noam Rosenthal
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Simon Fraser (smfr)
Comment 1
2020-01-28 14:44:45 PST
rdar://problem/58858225
Noam Rosenthal
Comment 2
2020-01-28 22:42:04 PST
(In reply to Simon Fraser (smfr) from
comment #0
)
>
r254406
broke the icons in the "star" column in Gmail. > > The ::before has: > content: -webkit-image-set('' 1x,'' 2x); > > It seems this was previously invalid, so that content:'' was used. But now > it's treated as valid, so we show the missing image icon. > > I think we need to preserve the old behavior of image-set in > -webkit-image-set.
or maybe disallow empty images?
Noam Rosenthal
Comment 3
2020-01-28 22:56:40 PST
(In reply to Noam Rosenthal from
comment #2
)
> (In reply to Simon Fraser (smfr) from
comment #0
) > >
r254406
broke the icons in the "star" column in Gmail. > > > > The ::before has: > > content: -webkit-image-set('' 1x,'' 2x); > > > > It seems this was previously invalid, so that content:'' was used. But now > > it's treated as valid, so we show the missing image icon. > > > > I think we need to preserve the old behavior of image-set in > > -webkit-image-set. > > or maybe disallow empty images?
I think I'll do both for now.
Noam Rosenthal
Comment 4
2020-01-29 00:52:37 PST
Created
attachment 389111
[details]
Patch
Simon Fraser (smfr)
Comment 5
2020-01-29 10:51:48 PST
(In reply to Noam Rosenthal from
comment #2
)
> (In reply to Simon Fraser (smfr) from
comment #0
) > >
r254406
broke the icons in the "star" column in Gmail. > > > > The ::before has: > > content: -webkit-image-set('' 1x,'' 2x); > > > > It seems this was previously invalid, so that content:'' was used. But now > > it's treated as valid, so we show the missing image icon. > > > > I think we need to preserve the old behavior of image-set in > > -webkit-image-set. > > or maybe disallow empty images?
Are empty images allowed per spec?
Simon Fraser (smfr)
Comment 6
2020-01-29 10:55:43 PST
Comment on
attachment 389111
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=389111&action=review
> Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:1706 > + if ((range.peek().type() == StringToken) && (allowedImageTypes & AllowedImageType::RawStringAsURL)) {
I think we prefer allowedImageTypes.contains(AllowedImageType::RawStringAsURL)
> Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:1714 > CSSValueID id = range.peek().functionId();
Not this patch, but please don't use 'id'; it's a reserved word in Objective-C.
> Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:1715 > + if ((allowedImageTypes & AllowedImageType::GeneratedImage) && isGeneratedImage(id))
.contains()
> Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:1718 > + if (allowedImageTypes & AllowedImageType::ImageSet) {
.contains()
> Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:1726 > + if (allowedImageTypes & AllowedImageType::URLFunction) {
.contains()
Noam Rosenthal
Comment 7
2020-01-30 04:27:51 PST
Created
attachment 389238
[details]
Patch for landing
Noam Rosenthal
Comment 8
2020-01-30 04:28:40 PST
(In reply to Simon Fraser (smfr) from
comment #5
)
> (In reply to Noam Rosenthal from
comment #2
) > > (In reply to Simon Fraser (smfr) from
comment #0
) > > >
r254406
broke the icons in the "star" column in Gmail. > > > > > > The ::before has: > > > content: -webkit-image-set('' 1x,'' 2x); > > > > > > It seems this was previously invalid, so that content:'' was used. But now > > > it's treated as valid, so we show the missing image icon. > > > > > > I think we need to preserve the old behavior of image-set in > > > -webkit-image-set. > > > > or maybe disallow empty images? > > Are empty images allowed per spec?
They actually are, considered empty URLs. Removed that part from the patch.
WebKit Commit Bot
Comment 9
2020-01-30 05:10:12 PST
Comment on
attachment 389238
[details]
Patch for landing Clearing flags on attachment: 389238 Committed
r255420
: <
https://trac.webkit.org/changeset/255420
>
WebKit Commit Bot
Comment 10
2020-01-30 05:10:14 PST
All reviewed patches have been landed. Closing bug.
Darin Adler
Comment 11
2020-02-01 16:35:55 PST
Did we check the behavior of other browsers in image-set? It would be unfortunate if we were the only one who matched the spec on this.
Noam Rosenthal
Comment 12
2020-02-02 01:18:28 PST
(In reply to Darin Adler from
comment #11
)
> Did we check the behavior of other browsers in image-set? It would be > unfortunate if we were the only one who matched the spec on this.
Of course, working on other browsers in parallel, currently getting the W3C tests around this to be comprehensive.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug