[Webkit-unassigned] [Bug 37215] Web Inspector: be more explicit about resource loading errors

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=37215





--- Comment #6 from Andrey Kosyakov <caseq at chromium.org>  2010-04-07 12:12:40 PST ---
(In reply to comment #5)
> (From update of attachment 52752 [details])
> 
> > +    String message = "Failed to load resource " + url;
> > +    if (status)
> > +        message += String::format(", status: %d", status);
> > +
> > +    addMessageToConsole(OtherMessageSource, LogMessageType, ErrorMessageLevel, message, 0, url);
> 
> I think it might be best to do this in the front-end, then the message can be
> localized. But that isn't as important.
> 

I did consider making it in front-end to enable localization, but it would be
an effort of a different scale -- we need either:
- introduce another call to front-end, along with some buffering for these
events in InspectorController in case front-end is not yet attached;
- introduce special console message type to piggy-back existing console message
transmission & buffering (a bit of a hack, perhaps).

I wasn't sure whether the added mess would be justified by increased
functionality, so I'd rather leave it to reviewers discretion ;)

> The message could be clearer, something like:
> 
>      Failed to load resource. The server responded with a status of %u.
>
> Consider using format for both variations of the message instead of appending
> strings. I don't think you need to include the URL in the message since the URL
> is automatically included as meta data and shown as a link with the message.

Agree, fixed.

> > +    void reportResourceError(const String& url, int status = 0);
> 
> Status should be unsigned.

I made it int as it happened to be int in ResourceResposeBase. Just being
paranoid about the unlikely case someone decides to (ab)use negative statuses
(e.g. for network-level errors?)

> > +        // failure.
> 
> It is unfornanate to have a widow in comments like this. It is less annoying to
> have longer lines than to have widows, IMHO.

Fixed.

> > -        resource.warnings = 0;
> > -        resource.errors = 0;
> 
> Why is this needed?

For XHRs, we happen to change resource type quite late (after
didReceiveResponse), and this used to reset errors/warnings counters. We only
call recreateViewForResourceIfNeeded in the only place, in Resource's setter
for category, so I thought resetting errors/warnings counters there is not
appropriate there and removing it wouldn't harm.

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