[webkit-reviews] review granted: [Bug 24024] REGRESSION (r39845): Assertion failure in -[WebHistoryItem dictionaryRepresentation] when archiving a submission to about:blank : [Attachment 27828] allow https, too

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Alexey Proskuryakov <ap at webkit.org>'s
request for review:
Bug 24024: REGRESSION (r39845): Assertion failure in -[WebHistoryItem
dictionaryRepresentation] when archiving a submission to about:blank
https://bugs.webkit.org/show_bug.cgi?id=24024

Attachment 27828: allow https, too
https://bugs.webkit.org/attachment.cgi?id=27828&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
>      if ([method length])
> -	   item->setLastVisitWasHTTPNonGet([method
caseInsensitiveCompare:@"GET"]);
> +	   item->setLastVisitWasHTTPNonGet([method
caseInsensitiveCompare:@"GET"] && (![[url scheme]
caseInsensitiveCompare:@"http"] || ![[url scheme]
caseInsensitiveCompare:@"https"]));

It doesn't make sense to me that some of this is in an if statement and some of
this is in the argument to the set function. We should either do it all one way
or all the other.

I also don't like multiple call sites that all include the "http"/"https" pair
and would really like to see a central function for this.

Alexey, I know you don't like the term "HTTP family", but I'd prefer a function
with a less concrete name than HTTPAndHTTPS.

r=me as-is, though, despite those comments