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

Hi Dave,

On Tue, Nov 21, 2017 at 3:52 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

> Hi
>
> On Tue, Nov 21, 2017 at 9:33 AM, Neel Patel <neel(dot)patel(at)enterprisedb(dot)com>
> wrote:
>
>> Hi Dave,
>>
>>
>> On Tue, Nov 21, 2017 at 2:36 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>>>
>>>
>>> On Tue, Nov 21, 2017 at 9:02 AM, Neel Patel <neel(dot)patel(at)enterprisedb(dot)com
>>> > wrote:
>>>
>>>> Hi Dave,
>>>>
>>>> On Mon, Nov 20, 2017 at 8:20 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>
>>>>> Hi
>>>>>
>>>>> On Mon, Nov 20, 2017 at 1:10 PM, Neel Patel <
>>>>> neel(dot)patel(at)enterprisedb(dot)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.webco
>>>> ntrols.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.
>

Ok. I will work on to fix this issue.

>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>