[webkit-reviews] review granted: [Bug 22194] REGRESSION: Dialog when going back to a page from whence you submitted a form : [Attachment 25907] Updated Changelog and Windows skipped list

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Dec 10 09:15:10 PST 2008


Darin Adler <darin at apple.com> has granted Brady Eidson <beidson at apple.com>'s
request for review:
Bug 22194: REGRESSION: Dialog when going back to a page from whence you
submitted a form
https://bugs.webkit.org/show_bug.cgi?id=22194

Attachment 25907: Updated Changelog and Windows skipped list
https://bugs.webkit.org/attachment.cgi?id=25907&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +		   // See https://bugs.webkit.org/show_bug.cgi?id=22194 for
more discussion

Should add a period here at the end of the sentence.

> +# Needs custom policy delegate enhancement in DRT

It's annoying to turn off so many tests on Windows. Is there a way to trigger
the new behavior only for your new test for now? How hard is it to do the
Windows side of this?

> +    switch (navType) {
> +	   case WebNavigationTypeLinkClicked:
> +	       typeDescription = "WebNavigationTypeLinkClicked";
> +	       break;
> +	   case WebNavigationTypeFormSubmitted:
> +	       typeDescription = "WebNavigationTypeFormSubmitted";
> +	       break;
> +	   case WebNavigationTypeBackForward:
> +	       typeDescription = "WebNavigationTypeBackForward";
> +	       break;
> +	   case WebNavigationTypeReload:
> +	       typeDescription = "WebNavigationTypeReload";
> +	       break;
> +	   case WebNavigationTypeFormResubmitted:
> +	       typeDescription = "WebNavigationTypeFormResubmitted";
> +	       break;
> +	   case WebNavigationTypeOther:
> +	       typeDescription = "WebNavigationTypeOther";
> +	       break;
> +	   default:
> +	       typeDescription = "Unknown";
> +    }

For the layout test results I'd suggest using strings that aren't part of the
Mac WebKit API, so "link clicked" rather than "WebNavigationTypeLinkClicked". I
think it might also make the test results easier to read.

I also think "Unknown" is slightly too mild when we get an constant value
that's not one of the defined ones. Since we already have "other", it's
particularly bad if we get a value that's not one of the legal values passed
in.

r=me as is; we can consider improvements in the testing machinery later, but
lets get the fix in right away


More information about the webkit-reviews mailing list