Bug 202712

Summary:

[iOS] REGRESSION (r250398) Layout Test fast/history/page-cache-element-state-focused.html is failing

Product:

WebKit

Reporter:

Chris Dumez <cdumez>

Component:

CSS

Assignee:

Chris Dumez <cdumez>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, darin, dino, esprehn+autocc, ews-watchlist, ggaren, glenn, gyuyoung.kim, koivisto, macpherson, menard, timothy, webkit-bug-importer, wenson_hsieh, zalan

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
WIP Patch	none
Patch	none

Description Chris Dumez 2019-10-08 16:59:23 PDT

fast/history/page-cache-element-state-focused.html is failing on internal bots.

Comment 1 Chris Dumez 2019-10-08 16:59:40 PDT

<rdar://problem/56082428>

Comment 2 Chris Dumez 2019-10-08 17:01:24 PDT

Created attachment 380481 [details]
WIP Patch

Needs change log.

Comment 3 Chris Dumez 2019-10-08 18:30:35 PDT

Created attachment 380490 [details]
Patch

Comment 4 WebKit Commit Bot 2019-10-09 07:38:52 PDT

Comment on attachment 380490 [details]
Patch

Clearing flags on attachment: 380490

Committed r250912: <https://trac.webkit.org/changeset/250912>

Comment 5 WebKit Commit Bot 2019-10-09 07:38:54 PDT

All reviewed patches have been landed.  Closing bug.

Comment 6 Darin Adler 2019-10-09 09:45:21 PDT

Comment on attachment 380490 [details]
Patch

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

> Source/WebCore/css/CSSDefaultStyleSheets.cpp:96
> +static const char* simpleUserAgentStyleSheet = "html,body,div{display:block}" CSS_DARK_MODE_ADDITION "head{display:none}body{margin:8px}div:focus,span:focus,a:focus{outline:auto " DEFAULT_OUTLINE_WIDTH " -webkit-focus-ring-color}a:any-link{color:-webkit-link;text-decoration:underline}a:any-link:active{color:-webkit-activelink}";

Tiny nitpick: There’s not enough const here. This declares a constant string, and a non-constant global variable that starts out pointing to it; but we don’t need to change the value of that global variable. We should add another const or constexpr so the pointer itself is constant too, not just the string contents. Or could use a constant array instead of a pointer to a string; I think that is a little smaller.

Comment 7 Chris Dumez 2019-10-09 10:27:35 PDT

(In reply to Darin Adler from comment #6)
> Comment on attachment 380490 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=380490&action=review
> 
> > Source/WebCore/css/CSSDefaultStyleSheets.cpp:96
> > +static const char* simpleUserAgentStyleSheet = "html,body,div{display:block}" CSS_DARK_MODE_ADDITION "head{display:none}body{margin:8px}div:focus,span:focus,a:focus{outline:auto " DEFAULT_OUTLINE_WIDTH " -webkit-focus-ring-color}a:any-link{color:-webkit-link;text-decoration:underline}a:any-link:active{color:-webkit-activelink}";
> 
> Tiny nitpick: There’s not enough const here. This declares a constant
> string, and a non-constant global variable that starts out pointing to it;
> but we don’t need to change the value of that global variable. We should add
> another const or constexpr so the pointer itself is constant too, not just
> the string contents. Or could use a constant array instead of a pointer to a
> string; I think that is a little smaller.

Landed a follow-up fix in <https://trac.webkit.org/changeset/250916> thanks.
I opted for "static const char simpleUserAgentStyleSheet[]".