[Webkit-unassigned] [Bug 37274] Report resource URL and error details in resource events

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Apr 9 10:31:29 PDT 2010


--- Comment #5 from Andrey Kosyakov <caseq at chromium.org>  2010-04-09 10:31:29 PST ---
(In reply to comment #4)
> (From update of attachment 52868 [details])
> > +    m_failingURL = failingURLString; 
> Good fix. Where's the test case for this, though? It's separate from the other
> fix. We don't take bug fixes without test cases.

There's a bit of a problem with testing for this -- currently, resource
notification related code in DumpRenderTree is platform-specific
(DumpRenderTree/mac/ResourceLoadDelegate.mm) and uses native platform classes
(NSError & co) only -- the classes that I changed are not used at all. Adding
tests for them at this level would require implementing tests explicitly for
different platforms, plus passing additional flags via LayoutTestsController. A
much cheaper and practical solution would be to test it on a higher level --
e.g. in inspector tests, as my initial motivation for fixing this was to enable
logging of messages to inspector console in case of resource loading problems.
This will also cover all platforms at once. This requires, however, that the
patch for logging resource errors to inspector is landed
(http://webkit.org/b/37215), so I added the test there. In will only pass once
both this and 37215 are landed, though.

> > -        // FIXME: it would be nice to have a way to get the real status text eventually.
> > -        m_httpStatusText = "OK";
> > +        // we used to return "OK" for everything, so be compatible and return "OK" for 200.
> > +        m_httpStatusText = m_httpStatusCode == 200 ? String("OK") 
> > +                                                   : String([NSHTTPURLResponse localizedStringForStatusCode: m_httpStatusCode]);
> Does the localizedStringForStatusCode method exist on all platforms we build
> on? Tiger?

At least the manual says so

Return Value
A localized string suitable for displaying to users that describes the
specified status code.

Available in Mac OS X v10.2 with Safari 1.0 installed.
Available in Mac OS X v10.2.7 and later.

> Formatting here is wrong -- we normally don't indent the second line of a
> conditional expression like this. We just indent by a single tab stop.


> What does [NSHTTPURLResponse localizedStringForStatusCode:] return for 200? Do
> we really need the hard-coded string?

localizedStringForStatusCode returns "no error". I don't have an evidence that
we really need this hack (except for a handful of tests that would need to be
re-baselined as a result of this), but considering this value is made available
to JS via XMLHttpRequest.statusText, I'd rather be on a safe side and leave it
as it used to be in cases where it doesn't contradict numeric value (especially
given that OK is what HTTP/1.1 suggests for 200).

Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

More information about the webkit-unassigned mailing list