Bug 61496 – [EFL] Add javascript_can_open_window API

• Component: WebKit EFL
• Status: RESOLVED WONTFIX

[Gyuyoung Kim]

Sometime user doesn't want to open new window for ad popup or unnecessary notification. So, I think it is better to add new API for that.

[Gyuyoung Kim]

Created attachment 94903 [details]
Patch

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/efl/ChangeLog', u'Source/Web..." exit_code: 1

Source/WebKit/efl/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 1 in 3 files


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

[Gyuyoung Kim]

Created attachment 94904 [details]
Patch

[Antonio Gomes]

Comment on attachment 94904 [details]
Patch

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

Leandro for final cq+

> Source/WebKit/efl/ewk/ewk_view.cpp:2986
> + * Sets the javascript can open new window.

It reads a bit strange.

[Antonio Gomes]

(In reply to comment #4)
> (From update of attachment 94904 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=94904&action=review
> 
> Leandro for final cq+
> 
> > Source/WebKit/efl/ewk/ewk_view.cpp:2986
> > + * Sets the javascript can open new window.
> 
> It reads a bit strange.

Err Demarchi, even ...

[Gyuyoung Kim]

Created attachment 94912 [details]
Patch

I modify the comment.

[Lucas De Marchi]

Comment on attachment 94912 [details]
Patch

Humn... I'm almost sure I implemented this ~ 1 year ago. Could you check if what you need is not what was done in r61706? I remember I was sending a signal to browser which could allow or deny the creation of a new window.

[Gyuyoung Kim]

(In reply to comment #7)
> (From update of attachment 94912 [details])
> Humn... I'm almost sure I implemented this ~ 1 year ago. Could you check if what you need is not what was done in r61706? I remember I was sending a signal to browser which could allow or deny the creation of a new window.

I think this patch is a little different from r61706. This patch prohibits that new window creation signal by javascript is not sent to ewk layer. But, it seems r61706 has similar feature.

If this patch is landed, I think there is no problem. But, this patch is a little duplicate with r61706.

Does you think this patch doesn't need to land ?

[Lucas De Marchi]

(In reply to comment #8)
> (In reply to comment #7)
> > (From update of attachment 94912 [details] [details])
> > Humn... I'm almost sure I implemented this ~ 1 year ago. Could you check if what you need is not what was done in r61706? I remember I was sending a signal to browser which could allow or deny the creation of a new window.
> 
> I think this patch is a little different from r61706. This patch prohibits that new window creation signal by javascript is not sent to ewk layer. But, it seems r61706 has similar feature.
> 

Is the net effect the same? I.e.: is blocking the signal any better than waiting for browser to allow or deny the window creation?


> If this patch is landed, I think there is no problem. But, this patch is a little duplicate with r61706.
> 
> Does you think this patch doesn't need to land ?

I think we shouldn't have duplicate features, but I'm ok if there's a reason to block the signal instead of waiting for the signal.

I remember that the signal treats the case with 'target="_blank"' too, so it seems to support more use cases. Is there any use case that your approach supports that is not supported by the current implementation?

[Gyuyoung Kim]

I can't find better pros than current implementation.