Bug 46134 – REGRESSION (r61285): AIM 2.1.296: Code rendered as text in Welcome screen

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Andy Estes]

After r61285, markup is displayed on the Mac AIM client's welcome screen when it links against a ToT WebKit.framework. This is due to a change in how WebKit's parser handles invalid markup. An app-specific workaround should be checked in.

[Andy Estes]

<rdar://problem/8154379>

[Andy Estes]

Created attachment 68172 [details]
Patch

Comment on attachment 68172 [details]
Patch

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

> WebKit/mac/WebView/WebView.mm:615
> +static bool shouldUsePreHTML5ParserQuirks(WebPreferences* preferences)
> +{
> +    return ([preferences usePreHTML5ParserQuirks] || applicationIsAOLInstantMessenger());
> +}

Should this be limited to certain versions of AIM or simply versions of AIM linked against versions of WebKit that had the old parser?

[Andy Estes]

(In reply to comment #3)
> (From update of attachment 68172 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=68172&action=review
> 
> > WebKit/mac/WebView/WebView.mm:615
> > +static bool shouldUsePreHTML5ParserQuirks(WebPreferences* preferences)
> > +{
> > +    return ([preferences usePreHTML5ParserQuirks] || applicationIsAOLInstantMessenger());
> > +}
> 
> Should this be limited to certain versions of AIM or simply versions of AIM linked against versions of WebKit that had the old parser?

bdash pointed out the rationale of doing this, so yes we should do this. I'm not sure whether it's better to check the version of AIM or check the version of WebKit AIM linked against; it seems the same thing can be accomplished either way.

(In reply to comment #4)
> (In reply to comment #3)
> > (From update of attachment 68172 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=68172&action=review
> > 
> > > WebKit/mac/WebView/WebView.mm:615
> > > +static bool shouldUsePreHTML5ParserQuirks(WebPreferences* preferences)
> > > +{
> > > +    return ([preferences usePreHTML5ParserQuirks] || applicationIsAOLInstantMessenger());
> > > +}
> > 
> > Should this be limited to certain versions of AIM or simply versions of AIM linked against versions of WebKit that had the old parser?
> 
> bdash pointed out the rationale of doing this, so yes we should do this. I'm not sure whether it's better to check the version of AIM or check the version of WebKit AIM linked against; it seems the same thing can be accomplished either way.

I would check the WebKit version, since AIM’s version numbering is out of my control (and doesn't make much sense). I am not even sure that this behavior should be restricted to AIM.

[Daniel Bates]

Comment on attachment 68172 [details]
Patch

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

> WebCore/ChangeLog:8
> +        No new tests. (OOPS!)

Briefly glanced at this patch and noticed this OOPS line. Obviously you will need to remove this line before landing.

[Andy Estes]

Created attachment 68189 [details]
Patch

[Adam Roben (:aroben)]

Comment on attachment 68189 [details]
Patch

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

> WebKit/mac/WebView/WebView.mm:621
> +static bool shouldUsePreHTML5ParserQuirks(WebPreferences* preferences)
> +{
> +    // AIM clients linked against versions of WebKit prior to the introduction
> +    // of the HTML5 parser contain markup incompatible with the new parser.
> +    // Enable parser quirks to remain compatible with these clients. See
> +    // <https://bugs.webkit.org/show_bug.cgi?id=46134>.
> +    static bool shouldUsePreHTML5ParserQuirks = [preferences usePreHTML5ParserQuirks]
> +        || (!WebKitLinkedOnOrAfter(WEBKIT_FIRST_VERSION_WITH_HTML5_PARSER) && applicationIsAOLInstantMessenger());
> +    return shouldUsePreHTML5ParserQuirks;
> +}

This doesn't seem to support a client application calling [preferences setUsePreHTML5ParserQuirks] after this function is first called, or applications that use multiple WebPreferences instances.

If you put the linked-on-or-after and is-AOL checks into their own static bool, then you can check [preferences usePreHTML5ParserQuirks] every time this function is called.

[Andy Estes]

Created attachment 68270 [details]
Patch

[Andy Estes]

(In reply to comment #8)
> (From update of attachment 68189 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=68189&action=review
> 
> > WebKit/mac/WebView/WebView.mm:621
> > +static bool shouldUsePreHTML5ParserQuirks(WebPreferences* preferences)
> > +{
> > +    // AIM clients linked against versions of WebKit prior to the introduction
> > +    // of the HTML5 parser contain markup incompatible with the new parser.
> > +    // Enable parser quirks to remain compatible with these clients. See
> > +    // <https://bugs.webkit.org/show_bug.cgi?id=46134>.
> > +    static bool shouldUsePreHTML5ParserQuirks = [preferences usePreHTML5ParserQuirks]
> > +        || (!WebKitLinkedOnOrAfter(WEBKIT_FIRST_VERSION_WITH_HTML5_PARSER) && applicationIsAOLInstantMessenger());
> > +    return shouldUsePreHTML5ParserQuirks;
> > +}
> 
> This doesn't seem to support a client application calling [preferences setUsePreHTML5ParserQuirks] after this function is first called, or applications that use multiple WebPreferences instances.
> 
> If you put the linked-on-or-after and is-AOL checks into their own static bool, then you can check [preferences usePreHTML5ParserQuirks] every time this function is called.

Yep, you're right. Fixed.

[Andy Estes]

Committed r67983: <http://trac.webkit.org/changeset/67983>