Bug 185458 – Use StyleColor::Options in more places

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Timothy Hatcher]

Using StyleColor::Options will avoid some confusing bools.

rdar://problem/39853798

[Timothy Hatcher]

Created attachment 339903 [details]
Patch

[Megan Gardner]

Comment on attachment 339903 [details]
Patch

R+ from me as well

[Timothy Hatcher]

Created attachment 339906 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 339906 [details]
Patch

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

> Source/WebCore/dom/Document.h:548
> +    bool defaultAppearance() const;

This function name is inscrutable. It sounds like it should return an "appearance" thing but returns a bool?

[Timothy Hatcher]

(In reply to Simon Fraser (smfr) from comment #4)
> Comment on attachment 339906 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=339906&action=review
> 
> > Source/WebCore/dom/Document.h:548
> > +    bool defaultAppearance() const;
> 
> This function name is inscrutable. It sounds like it should return an
> "appearance" thing but returns a bool?

This matches the name in other parts of the code. We should rename this everywhere in a separate step.

[Brent Fulgham]

Comment on attachment 339906 [details]
Patch

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

> Source/WebCore/platform/mac/LocalDefaultSystemAppearance.mm:51
> +    [NSAppearance setCurrentAppearance:m_savedSystemAppearance.get()];

We should be careful these calls are not happening in the WebContent process. We are locking down CGSWindowServer access, which may make these calls fail. They should be messaged back and forth to the UIProcess.

[Tim Horton]

(In reply to Brent Fulgham from comment #6)
> Comment on attachment 339906 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=339906&action=review
> 
> > Source/WebCore/platform/mac/LocalDefaultSystemAppearance.mm:51
> > +    [NSAppearance setCurrentAppearance:m_savedSystemAppearance.get()];
> 
> We should be careful these calls are not happening in the WebContent
> process. We are locking down CGSWindowServer access, which may make these
> calls fail. They should be messaged back and forth to the UIProcess.

Not accurate in this case.

[Timothy Hatcher]

Created attachment 339920 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 339920 [details]
Patch

Clearing flags on attachment: 339920

Committed r231557: <https://trac.webkit.org/changeset/231557>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/40092603>