Bug 62347 - [EFL] Add load error handler to EWebLauncher
Summary: [EFL] Add load error handler to EWebLauncher
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-08 19:54 PDT by Jaehun Lim
Modified: 2011-06-13 15:30 PDT (History)
7 users (show)

See Also:


Attachments
Proposed patch (2.20 KB, patch)
2011-06-08 20:02 PDT, Jaehun Lim
no flags Details | Formatted Diff | Diff
new patch (2.19 KB, patch)
2011-06-09 15:54 PDT, Jaehun Lim
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jaehun Lim 2011-06-08 19:54:36 PDT
Add a handler function of "load,error" to EWebLauncher.
EWebLauncher can show a simple error page when load error.
Comment 1 Jaehun Lim 2011-06-08 20:02:39 PDT
Created attachment 96530 [details]
Proposed patch
Comment 2 Leandro Pereira 2011-06-09 06:54:57 PDT
Comment on attachment 96530 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=96530&action=review

Just some minor nitpicks. Otherwise, LGTM.

> Tools/ChangeLog:9
> +        Add a handler function of "load,error" to EWebLauncher.
> +        EWebLauncher can show a simple error page when load error.

I think something along the lines of "EWebLauncher now displays a simple error page on load errors." would be better.

> Tools/EWebLauncher/main.c:321
> +    snprintf(message, 1024, "<html><body><div style=\"color:#ff0000\">ERROR !</div><br><div>Code : %d<br>Domain : %s<br>Description : %s<br>URL : %s</div></body</html>",

No spaces before punctuation. Use "ERROR!", "Code:", etc.
Comment 3 Jaehun Lim 2011-06-09 15:54:35 PDT
Created attachment 96657 [details]
new patch
Comment 4 Jaehun Lim 2011-06-09 15:57:04 PDT
(In reply to comment #2)
> (From update of attachment 96530 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=96530&action=review
> 
> Just some minor nitpicks. Otherwise, LGTM.
> 
> > Tools/ChangeLog:9
> > +        Add a handler function of "load,error" to EWebLauncher.
> > +        EWebLauncher can show a simple error page when load error.
> 
> I think something along the lines of "EWebLauncher now displays a simple error page on load errors." would be better.
> 
> > Tools/EWebLauncher/main.c:321
> > +    snprintf(message, 1024, "<html><body><div style=\"color:#ff0000\">ERROR !</div><br><div>Code : %d<br>Domain : %s<br>Description : %s<br>URL : %s</div></body</html>",
> 
> No spaces before punctuation. Use "ERROR!", "Code:", etc.

Thanks for your comments. I uploaded a new patch.
Comment 5 Gyuyoung Kim 2011-06-09 16:54:15 PDT
Internal r+ on my side.
Comment 6 Eric Seidel (no email) 2011-06-13 15:19:41 PDT
Comment on attachment 96657 [details]
new patch

rs=me.
Comment 7 Eric Seidel (no email) 2011-06-13 15:20:56 PDT
Comment on attachment 96657 [details]
new patch

View in context: https://bugs.webkit.org/attachment.cgi?id=96657&action=review

> Tools/EWebLauncher/main.c:322
> +    snprintf(message, 1024, "<html><body><div style=\"color:#ff0000\">ERROR!</div><br><div>Code: %d<br>Domain: %s<br>Description: %s<br>URL: %s</div></body</html>",
> +             err->code, err->domain, err->description, err->failing_url);

Do we have any concerns about security of this HTML injection?  I assume that the injected message can't be controleled by an attacker from a different domain?
Comment 8 WebKit Review Bot 2011-06-13 15:30:16 PDT
Comment on attachment 96657 [details]
new patch

Clearing flags on attachment: 96657

Committed r88714: <http://trac.webkit.org/changeset/88714>
Comment 9 WebKit Review Bot 2011-06-13 15:30:22 PDT
All reviewed patches have been landed.  Closing bug.