Bug 46134 - REGRESSION (r61285): AIM 2.1.296: Code rendered as text in Welcome screen
Summary: REGRESSION (r61285): AIM 2.1.296: Code rendered as text in Welcome screen
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Andy Estes
URL:
Keywords: InRadar, Regression
Depends on: 40961
Blocks:
  Show dependency treegraph
 
Reported: 2010-09-20 16:42 PDT by Andy Estes
Modified: 2010-09-21 13:44 PDT (History)
2 users (show)

See Also:


Attachments
Patch (4.62 KB, patch)
2010-09-20 18:34 PDT, Andy Estes
no flags Details | Formatted Diff | Diff
Patch (5.93 KB, patch)
2010-09-20 23:04 PDT, Andy Estes
no flags Details | Formatted Diff | Diff
Patch (5.90 KB, patch)
2010-09-21 11:57 PDT, Andy Estes
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andy Estes 2010-09-20 16:42:32 PDT
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.
Comment 1 Andy Estes 2010-09-20 16:43:28 PDT
<rdar://problem/8154379>
Comment 2 Andy Estes 2010-09-20 18:34:08 PDT
Created attachment 68172 [details]
Patch
Comment 3 mitz 2010-09-20 18:39:40 PDT
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?
Comment 4 Andy Estes 2010-09-20 18:52:15 PDT
(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.
Comment 5 mitz 2010-09-20 19:05:09 PDT
(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.
Comment 6 Daniel Bates 2010-09-20 21:55:05 PDT
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.
Comment 7 Andy Estes 2010-09-20 23:04:44 PDT
Created attachment 68189 [details]
Patch
Comment 8 Adam Roben (:aroben) 2010-09-21 06:37:53 PDT
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.
Comment 9 Andy Estes 2010-09-21 11:57:50 PDT
Created attachment 68270 [details]
Patch
Comment 10 Andy Estes 2010-09-21 11:58:22 PDT
(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.
Comment 11 Andy Estes 2010-09-21 13:44:11 PDT
Committed r67983: <http://trac.webkit.org/changeset/67983>