Thread: [pgAdmin4][runtime][patch]: Fix for RM#2679

Hi,

Please find attached patch to fix RM#2679.

Issue:-

Getting started links does not open second time from "Dashboard" panel if User close runtime tab and open any URL again.

Analysis:-

As in runtime Qt application, when user defined "target=_new" then "createWindow" signal is called but when user close that new windows and again click on link then "createWindow" signal is not getting called so from user point view nothing will happen.

Solution:-

To make it work in both runtime and web application, changed "target" attribute to "_blank" so that "createWindow" signal will be called every time when user click on any link.

Hi

On Mon, Nov 20, 2017 at 1:10 PM, Neel Patel <neel.patel@enterprisedb.com> wrote:

Hi,

Please find attached patch to fix RM#2679.

Issue:-

Getting started links does not open second time from "Dashboard" panel if User close runtime tab and open any URL again.

Analysis:-

As in runtime Qt application, when user defined "target=_new" then "createWindow" signal is called but when user close that new windows and again click on link then "createWindow" signal is not getting called so from user point view nothing will happen.

Solution:-

To make it work in both runtime and web application, changed "target" attribute to "_blank" so that "createWindow" signal will be called every time when user click on any link.

I think this is a partial workaround for the problem. We link to external sites such as postgresql.org - what happens if that tries to open something with target="_new"? It should be expected to work as well. I think we need to fix the underlying problem, not try to work around it. 

Yes. You are right. We can implement the actual underlying problem but curious to know the difference between "_blank" and "_new" target attribute.

I didn't find any reference document for target attribute value "_new". I searched below links.

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/A#attr-target

https://msdn.microsoft.com/en-us/library/system.web.ui.webcontrols.hyperlink.target(v=vs.110).aspx#Anchor_0

--

Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Hi Dave,

On Mon, Nov 20, 2017 at 8:20 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Mon, Nov 20, 2017 at 1:10 PM, Neel Patel <neel.patel@enterprisedb.com> wrote:

Hi,

Please find attached patch to fix RM#2679.

Issue:-

Getting started links does not open second time from "Dashboard" panel if User close runtime tab and open any URL again.

Analysis:-

As in runtime Qt application, when user defined "target=_new" then "createWindow" signal is called but when user close that new windows and again click on link then "createWindow" signal is not getting called so from user point view nothing will happen.

Solution:-

To make it work in both runtime and web application, changed "target" attribute to "_blank" so that "createWindow" signal will be called every time when user click on any link.

I think this is a partial workaround for the problem. We link to external sites such as postgresql.org - what happens if that tries to open something with target="_new"? It should be expected to work as well. I think we need to fix the underlying problem, not try to work around it. 

Yes. You are right. We can implement the actual underlying problem but curious to know the difference between "_blank" and "_new" target attribute.

I didn't find any reference document for target attribute value "_new". I searched below links.

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/A#attr-target

https://msdn.microsoft.com/en-us/library/system.web.ui.webcontrols.hyperlink.target(v=vs.110).aspx#Anchor_0

Thoughts ? 

On Tue, Nov 21, 2017 at 9:02 AM, Neel Patel <neel.patel@enterprisedb.com> wrote:

Hi Dave,

On Mon, Nov 20, 2017 at 8:20 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Mon, Nov 20, 2017 at 1:10 PM, Neel Patel <neel.patel@enterprisedb.com> wrote:

Hi,

Please find attached patch to fix RM#2679.

Issue:-

Getting started links does not open second time from "Dashboard" panel if User close runtime tab and open any URL again.

Analysis:-

As in runtime Qt application, when user defined "target=_new" then "createWindow" signal is called but when user close that new windows and again click on link then "createWindow" signal is not getting called so from user point view nothing will happen.

Solution:-

To make it work in both runtime and web application, changed "target" attribute to "_blank" so that "createWindow" signal will be called every time when user click on any link.

I think this is a partial workaround for the problem. We link to external sites such as postgresql.org - what happens if that tries to open something with target="_new"? It should be expected to work as well. I think we need to fix the underlying problem, not try to work around it. 

Yes. You are right. We can implement the actual underlying problem but curious to know the difference between "_blank" and "_new" target attribute.

I didn't find any reference document for target attribute value "_new". I searched below links.

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/A#attr-target

https://msdn.microsoft.com/en-us/library/system.web.ui.webcontrols.hyperlink.target(v=vs.110).aspx#Anchor_0

Thoughts ? 

Hmm, interesting. So, per the spec we should be using _blank - I'll commit the patch to fix that.

That means that _new *should* work as any other named target - open it if it doesn't exist and navigate within it.

In other words, I think we still have a bug here; using _new (much like using, say, "foo") should still open a new tab if an earlier one has been closed - much as a browser would.

--

Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Hi Dave,

On Tue, Nov 21, 2017 at 2:36 PM, Dave Page <dpage@pgadmin.org> wrote:

On Tue, Nov 21, 2017 at 9:02 AM, Neel Patel <neel.patel@enterprisedb.com> wrote:

Hi Dave,

On Mon, Nov 20, 2017 at 8:20 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Mon, Nov 20, 2017 at 1:10 PM, Neel Patel <neel.patel@enterprisedb.com> wrote:

Hi,

Please find attached patch to fix RM#2679.

Issue:-

Getting started links does not open second time from "Dashboard" panel if User close runtime tab and open any URL again.

Analysis:-

As in runtime Qt application, when user defined "target=_new" then "createWindow" signal is called but when user close that new windows and again click on link then "createWindow" signal is not getting called so from user point view nothing will happen.

Solution:-

To make it work in both runtime and web application, changed "target" attribute to "_blank" so that "createWindow" signal will be called every time when user click on any link.

I think this is a partial workaround for the problem. We link to external sites such as postgresql.org - what happens if that tries to open something with target="_new"? It should be expected to work as well. I think we need to fix the underlying problem, not try to work around it. 

Yes. You are right. We can implement the actual underlying problem but curious to know the difference between "_blank" and "_new" target attribute.

I didn't find any reference document for target attribute value "_new". I searched below links.

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/A#attr-target

https://msdn.microsoft.com/en-us/library/system.web.ui.webcontrols.hyperlink.target(v=vs.110).aspx#Anchor_0

Thoughts ? 

Hmm, interesting. So, per the spec we should be using _blank - I'll commit the patch to fix that.

Yes. We should use _blank instead of _new. 

That means that _new *should* work as any other named target - open it if it doesn't exist and navigate within it.

Yes - As per my knowledge, "target=_new" means URL to be opened in new frame with name="_new".

In other words, I think we still have a bug here; using _new (much like using, say, "foo") should still open a new tab if an earlier one has been closed - much as a browser would.

As per the docs, we should use "_blank". Do you feel we should fix in runtime as well for _new, If yes - I can work on that.

Hi

On Tue, Nov 21, 2017 at 9:33 AM, Neel Patel <neel.patel@enterprisedb.com> wrote:

Hi Dave,

On Tue, Nov 21, 2017 at 2:36 PM, Dave Page <dpage@pgadmin.org> wrote:

On Tue, Nov 21, 2017 at 9:02 AM, Neel Patel <neel.patel@enterprisedb.com> wrote:

Hi Dave,

On Mon, Nov 20, 2017 at 8:20 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Mon, Nov 20, 2017 at 1:10 PM, Neel Patel <neel.patel@enterprisedb.com> wrote:

Hi,

Please find attached patch to fix RM#2679.

Issue:-

Getting started links does not open second time from "Dashboard" panel if User close runtime tab and open any URL again.

Analysis:-

As in runtime Qt application, when user defined "target=_new" then "createWindow" signal is called but when user close that new windows and again click on link then "createWindow" signal is not getting called so from user point view nothing will happen.

Solution:-

To make it work in both runtime and web application, changed "target" attribute to "_blank" so that "createWindow" signal will be called every time when user click on any link.

I think this is a partial workaround for the problem. We link to external sites such as postgresql.org - what happens if that tries to open something with target="_new"? It should be expected to work as well. I think we need to fix the underlying problem, not try to work around it. 

Yes. You are right. We can implement the actual underlying problem but curious to know the difference between "_blank" and "_new" target attribute.

I didn't find any reference document for target attribute value "_new". I searched below links.

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/A#attr-target

https://msdn.microsoft.com/en-us/library/system.web.ui.webcontrols.hyperlink.target(v=vs.110).aspx#Anchor_0

Thoughts ? 

Hmm, interesting. So, per the spec we should be using _blank - I'll commit the patch to fix that.

Yes. We should use _blank instead of _new. 

Committed.

 

That means that _new *should* work as any other named target - open it if it doesn't exist and navigate within it.

Yes - As per my knowledge, "target=_new" means URL to be opened in new frame with name="_new".

Right - and "_new" could be any string at all. It doesn't have special meaning like _blank.

 

In other words, I think we still have a bug here; using _new (much like using, say, "foo") should still open a new tab if an earlier one has been closed - much as a browser would.

As per the docs, we should use "_blank". Do you feel we should fix in runtime as well for _new, If yes - I can work on that.

Right - _blank fixes the initial bug, however _new (or anything else) should also open a new tab if needed. We may not use that in pgAdmin itself, but the sites the app links to might. 

--

Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company