[webkit-reviews] review denied: [Bug 7171] No description in WebKitErrors.m for WebKitErrorPlugInWillHandleLoad : [Attachment 6380] Patch v1

bugzilla-request-daemon at opendarwin.org bugzilla-request-daemon at opendarwin.org
Fri Feb 10 09:54:51 PST 2006


John Sullivan <sullivan at apple.com> has denied Darin Adler <darin at apple.com>'s
request for review:
Bug 7171: No description in WebKitErrors.m for WebKitErrorPlugInWillHandleLoad
http://bugzilla.opendarwin.org/show_bug.cgi?id=7171

Attachment 6380: Patch v1
http://bugzilla.opendarwin.org/attachment.cgi?id=6380&action=edit

------- Additional Comments from John Sullivan <sullivan at apple.com>
This particular error code is an unusual one in that it doesn't really signify
an error, but the rest of the code behaves as if an error occurred. Currently
Safari goes out of its way to show no error text at all for this case. But this
still isn't quite right, because the status bar and Activity window still count
the error, so you see "1 error" when the load is actually successful.

This broader problem was in Radar, and I've just added it to Bugzilla as bug
7176.

These descriptions generally appear in Safari in the Activity window (but this
one is special-cased to not appear, as I said). They also appear in Safari in
the page that's displayed when a URL can't be reached, if Safari has not
supplied a specific message for that error number. In this particular case, I
believe that WebKitErrorDescriptionPlugInWillHandleLoad doesn't occur for main
pages, so the error description would never appear in the latter context.

So the new error description here would never appear in Safari. However, it
might appear in other WebKit clients. Depending on whether and how 7176 is
eventually addressed, this error code might be eliminated. However, in the
meantime I see no harm in providing a string here, for the benefit of clients
other than Safari. Brevity really pays off for these messages since they might
be displayed in a context (e.g. Activity window) where space is tight, so I'd
suggest "Plug-in handled load" rather than "Plug-in will handle load". I'm not
that excited about using "load" here as a noun, but I can't think of anything
obviously better.

I see Darin's point that the localizer comments could be improved. Currently
they are all of the form "WebKitErrorXXX description". They would be better if
they clarified when this type of error occurred, but only if this can be done
briefly and without just repeating the words that are already part of the error
name. I don't think doing that is necessary for committing this bug fix.

Also, since this modifies localized strings, you need to run
update-webkit-localizable-strings and patch Localizable.strings as well as this
file. 

So review- for now just to get the shorter string and to run
update-webkit-localizable-strings.



More information about the webkit-reviews mailing list