Bug 93828 – [EFL][WK2][Minibrowser] Support html link of blank target attribute.

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: WebKit EFL

[Kangil Han]

Supporting html link of blank target attribute would be needed for verifying basic webkit behavior in minibrowser.

[Kangil Han]

Created attachment 157972 [details]
patch

[WebKit Review Bot]

Attachment 157972 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/MiniBrowser/efl/..." exit_code: 1
Tools/MiniBrowser/efl/main.c:184:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Tools/MiniBrowser/efl/main.c:184:  An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
Tools/MiniBrowser/efl/main.c:195:  Missing space after ,  [whitespace/comma] [3]
Tools/MiniBrowser/efl/main.c:209:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 4 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Kangil Han]

Created attachment 157981 [details]
patch

[Kangil Han]

formal/informal review please!

[Raphael Kubo da Costa (:rakuco)]

Comment on attachment 157981 [details]
patch

Erm, needing to fork() just launch a new window does not smell very well. I haven't been watching the development of the WK2 port very closely, though. Are you sure there isn't a saner way to do this?

[Kangil Han]

(In reply to comment #5)
> (From update of attachment 157981 [details])
> Erm, needing to fork() just launch a new window does not smell very well. I haven't been watching the development of the WK2 port very closely, though. Are you sure there isn't a saner way to do this?

Sorry, I don't have an idea at a moment. :-(
I will ask more formal/informal reviews. ;-)

[Kangil Han]

ryuan: informal review please?

[Ryuan Choi]

Comment on attachment 157981 [details]
patch

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

> Tools/MiniBrowser/efl/main.c:203
> +    pid = fork();
> +    if (!pid) // child process
> +        execl(cmd, cmd, url, (char *)NULL);
> +    else if (pid < 0) // failed to fork
> +        fprintf(stderr, "Failed to fork\n");

You don't need to fork process.
Instead, create new evas and ewk_view.

> Tools/MiniBrowser/efl/main.c:253
> +    evas_object_smart_callback_add(app->browser, "policy,decision,new,window", on_new_window, app);

If I am right, policy,decision,new,window is for creating window.

You can call create,window

[Ryuan Choi]

(In reply to comment #8)
> (From update of attachment 157981 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=157981&action=review
> 
> > Tools/MiniBrowser/efl/main.c:203
> > +    pid = fork();
> > +    if (!pid) // child process
> > +        execl(cmd, cmd, url, (char *)NULL);
> > +    else if (pid < 0) // failed to fork
> > +        fprintf(stderr, "Failed to fork\n");
> 
> You don't need to fork process.
> Instead, create new evas and ewk_view.
> 
> > Tools/MiniBrowser/efl/main.c:253
> > +    evas_object_smart_callback_add(app->browser, "policy,decision,new,window", on_new_window, app);
> 
> If I am right, policy,decision,new,window is for creating window.
"is not for"
> 
> You can call create,window

[Jinwoo Song]

Kangil, can I take over this bug?

[Kangil Han]

(In reply to comment #10)
> Kangil, can I take over this bug?

Sure!

[Jinwoo Song]

This bug is no more valid as the following patch addressed this issue.
http://trac.webkit.org/changeset/130660

*** This bug has been marked as a duplicate of bug 98667 ***