Bug 135072

Summary: [iOS] Tapping "Allow Website" on a restricted page does not bring up the keypad
Product: WebKit Reporter: Andy Estes <aestes>
Component: New BugsAssignee: Andy Estes <aestes>
Status: RESOLVED FIXED    
Severity: Normal CC: beidson, ddkilzer, sam, thorton
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch ddkilzer: review+

Description Andy Estes 2014-07-18 14:52:18 PDT
[iOS] Tapping "Allow Website" on a restricted page does not bring up the keypad
Comment 1 Andy Estes 2014-07-18 15:09:19 PDT
Created attachment 235149 [details]
Patch
Comment 2 David Kilzer (:ddkilzer) 2014-07-18 16:36:42 PDT
Comment on attachment 235149 [details]
Patch

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

r=me

> Source/WebCore/platform/mac/ContentFilterMac.mm:250
> +    @try {
> +        if ([getWebFilterEvaluatorClass() conformsToProtocol:@protocol(NSSecureCoding)])
> +            contentFilter.m_platformContentFilter = (WebFilterEvaluator *)[unarchiver decodeObjectOfClass:getWebFilterEvaluatorClass() forKey:platformContentFilterKey];
> +        return true;
> +    } @catch (NSException *exception) {

Do we really want to return true here if we fall through the if statement?  Seems like we'd want something like this:

        if ([getWebFilterEvaluatorClass() conformsToProtocol:@protocol(NSSecureCoding)]) {
            contentFilter.m_platformContentFilter = (WebFilterEvaluator *)[unarchiver decodeObjectOfClass:getWebFilterEvaluatorClass() forKey:platformContentFilterKey];
            return true;
        }
Comment 3 Andy Estes 2014-07-18 17:04:34 PDT
(In reply to comment #2)
> (From update of attachment 235149 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=235149&action=review
> 
> r=me
> 
> > Source/WebCore/platform/mac/ContentFilterMac.mm:250
> > +    @try {
> > +        if ([getWebFilterEvaluatorClass() conformsToProtocol:@protocol(NSSecureCoding)])
> > +            contentFilter.m_platformContentFilter = (WebFilterEvaluator *)[unarchiver decodeObjectOfClass:getWebFilterEvaluatorClass() forKey:platformContentFilterKey];
> > +        return true;
> > +    } @catch (NSException *exception) {
> 
> Do we really want to return true here if we fall through the if statement?  Seems like we'd want something like this:
> 
>         if ([getWebFilterEvaluatorClass() conformsToProtocol:@protocol(NSSecureCoding)]) {
>             contentFilter.m_platformContentFilter = (WebFilterEvaluator *)[unarchiver decodeObjectOfClass:getWebFilterEvaluatorClass() forKey:platformContentFilterKey];
>             return true;
>         }

I believe we do want to return true, because WebFilterEvaluator not conforming to NSSecureCoding does not represent a failure to decode the message, just that we are running against an older version of the framework implementing that class.
Comment 4 Andy Estes 2014-07-18 17:16:44 PDT
Brady also looked at this and gave me an okay on IRC.
Comment 5 Andy Estes 2014-07-18 17:19:50 PDT
Committed r171252: <http://trac.webkit.org/changeset/171252>
Comment 6 Andy Estes 2014-07-18 17:20:21 PDT
<rdar://problem/17528188>