Hi Professor,

I am not sure if this is a known issue, but When I try to edit any wiki page and click on save, the page is being saved but the user is being redirected to the Main page of the group.

example : http://yioop.com/?c=admin&YIOOP_TOKEN=TAXXWVGWb44|1415955426&group_id=3&arg=edit&a=wiki&page_name=Browse%20Groups&back_params[open_help_page]=Browse%20Groups&back_params[c]=admin&back_params[a]=manageGroups

I am observing this issue after the doMessage redirect code has been committed.

Thanks Eswara.

Steps to reproduce: Goto : http://yioop.com/?c=admin&YIOOP_TOKEN=TAXXWVGWb44|1415955426&group_id=3&arg=edit&a=wiki&page_name=Browse%20Groups&back_params[open_help_page]=Browse%20Groups&back_params[c]=admin&back_params[a]=manageGroups

- Edit the article and click on Save button.

- Expected - redirect to read page.

- Actual - redirect to main page of Group.

Should be fixed by 3fa1acc. If not, reopen.

Hi Professor,

The issue where the page was redirecting to "Main" page was fixed. However There is another issue I found with back button that was broken due to redirectWithMessage. I am reopening this issue as they are similar.

When a user originates to the Edit page from help open page, and he edits and then clicks save, the back button is missing. The "back params" list is sent as post params via the edit form. However when the redirectWithMessage is called (Page Saved message) , the backparams are not passed along with the $location where the user is headed to. the $loaction in redirectWithMessage is computed cleanly. This results in the back_button being removed when the used edits the changes to help article.

Can you suggest me If I can concatenate the back_params to $location if back_params exist? I know You dont like the back logic to be in controller. but as the page is being redirected , controller is the final location that the user visits, please advise on where I can set the back params.

Thanks Eswara

Hi Professor,

I fixed a few things with the back button to work with the new redirectWithMessage changes, please take a look at the patch. Basically, the redirectWithMessage assumes all the fields sent(copy_fields) to be string, where as back_params is an array. I made a fix to take care of array params as well.

back_button_fixes.patch

edit- replaced the patch with latest changes pulled. edit2- just replaced the patch with all amended fixes. Please take a look at the changes. I just tested the patch and it works with all use cases of user navigating with back link.

Thanks Eswara

Hi Eswara,

I've applied and push your patch. I am a little bit concerned with how your back button code works. It seems to grab too many $_REQUEST parameters. For example, it grabs a bunch of things set by MAMP but not by Yioop in my test set-up and sticks them all into the url: http://localhost/git/yioop/?open_help_page=Machine+Information&c=admin&a=manageMachines&utag_main=_st%3A1405707580699%24ses_id%3A1405705971674%3Bexp-session&s_fid=31120108D9A6AC91-211EDA5A4327CBF4&yioopbiscuit=c7f999043ebd48c6265064ea13d022f7&has_queue_server=&parent=&YIOOP_TOKEN=dAENEYju378|1416004178 For example, utag_main and yioopbiscuit are cookie info and so your code might have security issues. Another problem is that if you go Manage Machine, click on the help next to Machine Information, click edit , click back, then the machine status update shows the wiki page rather than the status. This happens about 50% of the time for me.

Hi Professor,

Regarding additional REQUEST params being added, I am not sure If the difference in environment is the problem, but I dont have this problem with XAMPP, Also I am not sure why cookie params(like yioopbiscuit) are getting added to the $_REQUEST super global. I will test by installing MAMP to see why this is happening.

For the machine status issue, seems like the machineStatusUpdate function is delayed when the open help is executed. I will check the issue.

Eswara.

Hi Professor,

I fixed the machineStatusUpdate issue. for the other issue, I think $_COOKIE params need to be filtered out of $_REQUEST params, while constructing the back_url. Is it ok If i Use cleaned $_GET + $_POST params vs using $_REQUEST params? If yes, I will create a new patch with both the fixes. If not, i will research an alternative solution.

Thanks Eswara

I don't like solutions which are willy-nilly grabbing all parameters from the super globals. Please try to restrict to things that yioop itself sends.

Hi Professor,

I am actually working on an alternative solution where we capture specifically only the params we want to be included in back url, which gives more control. I will add another patch as soon as I am done, deleting the old patch(and note) for now.

Eswara.

Hi Professor,

Can you please take a look at my new patch which fixes both the issues, and please let me know your feedback.

Please see back_button_fixes_v2.patch

I made sure that we are adding the params only sent by Yioop to be added to back params list.

Eswara

The name backParams doesn't follow Yioop naming conventions, it should be back_params. Can you briefly explain the re-ordering of the assignment in web_layout?

I will make the change to back_params.

The function call to draw machine status is being delayed when help is opened after the machine-status function is called. So I made the reassignment sk that help is opened , and then machinestatus is drawn after that. Although I am not able to figureout what exactly is interfereing with the machinestatus from being delayed, if it was called before help is opened.

Hi Professor,

I have changed the backParams -> back_params.

Also there was an issue with page not redirecting to read mode, on wiki save. this was because the 'arg' was also passed to redirect URL after showing message for saving wiki articles.

Please find back_button_fixes_v3.patch

Thanks Eswara

Hi Eswara,

The arg was deliberate. In practice, when editing large documents the redirect to read is very awkward. So I decided to go back to the old way.

Chris

Hi Professor,

I have replaced the patch to revert the behaviour to the old way of Edit -> save ->Edit.

Please see back_button_fixes_v3.patch.

Eswara

Hi professor,

I think the JS is broken due to the commit f501f6a0d3ff1b4ee8ba7bf48acd8035b3636341. Problem seems to be this declaration : function setDisplay(id, value, display_type="block")

I know this holds good in PHP , but in JS this is not allowed I think. Can I change this to :

if(display_type === undefined){

       display_type = "block";

}

Please advise.

[Edit] - actually to be safe, compare with undefined and set it to block.

Eswara.

Professor,

I added a new patch including the new commit for JS fix. Please see

back_button_fixes_v4.patch

Eswara

It is in Ecmascript 6 which is supported by Firefox and KDE but I guess not elsewhere. So it was working for me, but we need to be more cross-platform so it is good you caught it. I have pushed your patch.

Ah, ok. Good to know. I will read up a little bit on ES6.

Also If everything is good, You can go ahead and close this issue.

Thanks Eswara.

Take 2 with 3ae8b85

Attachments:

back_button_fixes.patch

back_button_fixes_v4.patch

back_button_fixes_v2.patch

back_button_fixes_v3.patch