Bug 120173 – [iOS] Upstream changes to WebCore/style

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Daniel Bates]

Upstream iOS-specific changes to WebCore/style. We should look to clean up and find a better place for the content observer code.

[Daniel Bates]

Created attachment 209395 [details]
Patch

[Darin Adler]

Comment on attachment 209395 [details]
Patch

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

Looks fine. The merged code isn’t great, but we can improve it later; good to have it upstreamed as a start.

> Source/WebCore/style/StyleResolveTree.cpp:474
> +    CheckForVisibilityChangeOnRecalcStyle checkForVisibilityChange(current, current->renderStyle());

Seems like a strange way to plug this in. Maybe we can come up with something better or at least explain why this is platform-specific.

> Source/WebCore/style/StyleResolveTree.cpp:542
> +            if (settings->fontFallbackPrefersPictographs() && document->styleResolverIfExists())
> +                documentStyle->font().update(document->styleResolverIfExists()->fontSelector());

Would be nice to put style resolver into a local rather than evaluating it twice.

[Daniel Bates]

(In reply to comment #2)
> [...]
> > Source/WebCore/style/StyleResolveTree.cpp:474
> > +    CheckForVisibilityChangeOnRecalcStyle checkForVisibilityChange(current, current->renderStyle());
> 
> Seems like a strange way to plug this in. Maybe we can come up with something better or at least explain why this is platform-specific.

I agree. Filed bug #120478 to fix this issue.

> 
> > Source/WebCore/style/StyleResolveTree.cpp:542
> > +            if (settings->fontFallbackPrefersPictographs() && document->styleResolverIfExists())
> > +                documentStyle->font().update(document->styleResolverIfExists()->fontSelector());
> 
> Would be nice to put style resolver into a local rather than evaluating it twice.

Will fix before landing.

[Daniel Bates]

Committed r154820: <http://trac.webkit.org/changeset/154820>