summary: prompts about leaving the page after updating row data from browse tab --> (ok 4.2.3) prompts about leaving the page after updating row data from browse tab
status: open --> resolved
Group: Latest_Git --> 4.2.2
Priority: 5 --> 1
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Hi,
I think this bug was a consequence of "RFE #1518 Confirm dialog on accidentally leaving a page" which is yet to be released in v4.3.0. This bug was not present in v4.2 i.e. it should have been merged in master instead of QA_4_2.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
summary: (ok 4.2.3) prompts about leaving the page after updating row data from browse tab --> (ok 4.3) about leaving the page after updating row data from browse tab
status: resolved --> fixed
Group: 4.2.2 --> Latest_Git
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Yes, I tested again after clearing the cache and then closed and reopened web browser and also tested in different web browser and still getting the prompt.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I get the prompt when clicking on a menu tab after I do an unsaved change from the Edit page (which is correct IMO), but I don't get a prompt if I don't have an unsaved change.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Hi Chirayu,
I think getting the prompt is what we want if user has not saved the changes.
I din't get what you mean to say by "still getting the prompt".
Thanks
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Are you updating some value using inline grid editing? Are you testing with $cfg['SaveCellsAtOnce'] as true?
Here is the steps I followed,
1) Browse Table tab
2) use Inline grid editing and update some value. ($cfg['SaveCellsAtOnce'] is false, so it will be saved instantly).
3) Clicked a link in navigation panel.
Last edit: Chirayu Chiripal 2014-06-01
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Hi Shivam,
We already have a mechanism which prompts us when we are leaving a page without saving changes (RFE #1518). This feature is currently in master branch not in QA_4_2 branch. This bug says that even if we have saved the changes (see steps mentioned in my comment previously) we still get the message prompt about leaving the page which is not desired because we have saved the changes already. Also, test it with $cfg['SaveCellsAtOnce'] = false in config.inc.php or by unchecking "Grid editing: save all edited cells at once" value from settings > main panel > browse mode.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Hi Chirayu,
I'm sorry for the duplication of the patch and for misunderstanding this bug. I was not aware that this functionality has been already implemented before by Minhaz. I went through all the previous pull request and comments on this issue and now everything is clear to me. There are few things that I would like to bring it into notice :
If $cfg['SaveCellsAtOnce'] = false, and we do grid editing it is automatically saved. There is NO prompt which is being displayed in my patch. i.e The above mentioned bug does not exists. The reason is simple, I'm registering an event handler only "insertForm" not on "resultForm" or any other form. The reason you are still able to reproduce the bug is that first you will need to revert the previous patch submitted by Minhaz i.e in ajax.js
(https://github.com/phpmyadmin/phpmyadmin/pull/1152). It is still there in the master hence causing the bug to be reproduced.
In the older patch I encountered few more bugs like, when I click on any table and enter some text in "Filter rows:" and then try to navigate away, it will display the prompt which is not desired. (Refer attachment)
Older patch din't work for page refresh / page closed events. I've made one more commit into my branch and this problem is also fixed.
I feel the older patch is bit complex (for loop etc) and the problem can be handled in more simplified way.
What you guys think ? Shall I work on the previous patch ? Or make another pull request to master with my new patch ? ( I've added a handler for page refresh/close also)
Hi shivam,
maybe you have created your branch bug_4415 from QA_4_2 branch and not from master due to which you are not able to reproduce the bug in your patch. We intend to solve this bug in master, so you have to rebase your branch to master.
As far as reverting minhaz commit is concerned, I am not sure about it because this feature is applied to all pages of the phpMyAdmin (Insert/Edit, Create Table/Database, etc) and is not only limited to browse tab. Also, if we revert that feature then there is no need to fix this bug as it will not be present anymore but that's not a good idea in my opinion. Actually, I am also new to this community and I think we need a help here from other developers and maybe we need to bring minhaz in here because he has a better knowledge about how this feature works.
For bugs having milestone Latest_Git, you should check whether same bug exists in latest QA (i.e here QA_4_2) branch also apart from master branch. If that bug exists in QA branch then you should start your fix branch from QA branch and send a pull request to the same. If bug is only present in master branch then you should start your fix branch from master and send a pull request to master branch. Marc, please correct me if I am wrong anywhere.
And thanks for finding few more bugs with it. I can also reproduce the bug with "Filter Rows:".
Thanks,
Chirayu
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Sure. We will speak to Minhaz, he will help us on this.
I feel the patch he sent is too generic and that's why other bugs are arising because of it. In my branch I added event handler for Insert/Edit only, similarly we can have few more handlers. I don't think we will require many.
Marc, will you please guide us on this ?
And thanks for the info, I will keep it in mind while creating branch next time.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Fixed.
https://github.com/phpmyadmin/phpmyadmin/pull/1202
Hi,
I think this bug was a consequence of "RFE #1518 Confirm dialog on accidentally leaving a page" which is yet to be released in v4.3.0. This bug was not present in v4.2 i.e. it should have been merged in master instead of QA_4_2.
Moreover, I pulled the patch into master branch and I can still reproduce it.
Hi Chirayu,
you're right; I think I'll revert in QA_4_2.
But I cannot reproduce in master. Did you clear your cache and reload your page?
Yes, I tested again after clearing the cache and then closed and reopened web browser and also tested in different web browser and still getting the prompt.
I get the prompt when clicking on a menu tab after I do an unsaved change from the Edit page (which is correct IMO), but I don't get a prompt if I don't have an unsaved change.
Hi Chirayu,
I think getting the prompt is what we want if user has not saved the changes.
I din't get what you mean to say by "still getting the prompt".
Thanks
Are you updating some value using inline grid editing? Are you testing with $cfg['SaveCellsAtOnce'] as true?
Here is the steps I followed,
1) Browse Table tab
2) use Inline grid editing and update some value. ($cfg['SaveCellsAtOnce'] is false, so it will be saved instantly).
3) Clicked a link in navigation panel.
Last edit: Chirayu Chiripal 2014-06-01
Thanks Chirayu for these new steps; maybe Shivam can work on this.
Hi Shivam,
We already have a mechanism which prompts us when we are leaving a page without saving changes (RFE #1518). This feature is currently in master branch not in QA_4_2 branch. This bug says that even if we have saved the changes (see steps mentioned in my comment previously) we still get the message prompt about leaving the page which is not desired because we have saved the changes already. Also, test it with $cfg['SaveCellsAtOnce'] = false in config.inc.php or by unchecking "Grid editing: save all edited cells at once" value from settings > main panel > browse mode.
Hi Chirayu,
I'm sorry for the duplication of the patch and for misunderstanding this bug. I was not aware that this functionality has been already implemented before by Minhaz. I went through all the previous pull request and comments on this issue and now everything is clear to me. There are few things that I would like to bring it into notice :
If $cfg['SaveCellsAtOnce'] = false, and we do grid editing it is automatically saved. There is NO prompt which is being displayed in my patch. i.e The above mentioned bug does not exists. The reason is simple, I'm registering an event handler only "insertForm" not on "resultForm" or any other form. The reason you are still able to reproduce the bug is that first you will need to revert the previous patch submitted by Minhaz i.e in ajax.js
(https://github.com/phpmyadmin/phpmyadmin/pull/1152). It is still there in the master hence causing the bug to be reproduced.
In the older patch I encountered few more bugs like, when I click on any table and enter some text in "Filter rows:" and then try to navigate away, it will display the prompt which is not desired. (Refer attachment)
Older patch din't work for page refresh / page closed events. I've made one more commit into my branch and this problem is also fixed.
I feel the older patch is bit complex (for loop etc) and the problem can be handled in more simplified way.
What you guys think ? Shall I work on the previous patch ? Or make another pull request to master with my new patch ? ( I've added a handler for page refresh/close also)
My branch : https://github.com/shivamdixit/phpmyadmin/tree/bug_4415
Thanks
Shivam
Last edit: Shivam Dixit 2014-06-02
Hi shivam,
maybe you have created your branch bug_4415 from QA_4_2 branch and not from master due to which you are not able to reproduce the bug in your patch. We intend to solve this bug in master, so you have to rebase your branch to master.
As far as reverting minhaz commit is concerned, I am not sure about it because this feature is applied to all pages of the phpMyAdmin (Insert/Edit, Create Table/Database, etc) and is not only limited to browse tab. Also, if we revert that feature then there is no need to fix this bug as it will not be present anymore but that's not a good idea in my opinion. Actually, I am also new to this community and I think we need a help here from other developers and maybe we need to bring minhaz in here because he has a better knowledge about how this feature works.
For bugs having milestone Latest_Git, you should check whether same bug exists in latest QA (i.e here QA_4_2) branch also apart from master branch. If that bug exists in QA branch then you should start your fix branch from QA branch and send a pull request to the same. If bug is only present in master branch then you should start your fix branch from master and send a pull request to master branch. Marc, please correct me if I am wrong anywhere.
And thanks for finding few more bugs with it. I can also reproduce the bug with "Filter Rows:".
Thanks,
Chirayu
Sure. We will speak to Minhaz, he will help us on this.
I feel the patch he sent is too generic and that's why other bugs are arising because of it. In my branch I added event handler for Insert/Edit only, similarly we can have few more handlers. I don't think we will require many.
Marc, will you please guide us on this ?
And thanks for the info, I will keep it in mind while creating branch next time.
Shivam,
we'll discuss this at our IRC team meeting next week.
Sure Marc. Thanks.
Meanwhile is there any other high priority bug that I may look into ?
Shivam,
we have various "token mismatch" bug tickets. You can have a look at this one: https://sourceforge.net/p/phpmyadmin/bugs/4428/ to see if you are able to reproduce.
For a different kind of bug, have a look at https://sourceforge.net/p/phpmyadmin/bugs/4410/.
PR: https://github.com/phpmyadmin/phpmyadmin/pull/1222