Bug 62347

Summary: [EFL] Add load error handler to EWebLauncher
Product: WebKit Reporter: Jaehun Lim <ljaehun.lim>
Component: WebKit EFLAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: gyuyoung.kim, gyuyoung.kim, kenneth, lucas.de.marchi, rakuco, tonikitoo, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Linux   
Attachments:
Description Flags
Proposed patch
none
new patch none

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.