New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Media Manager still not overwrite existing files (2017-02-19a) #1864

Closed
cwchien opened this Issue Feb 22, 2017 · 17 comments

Comments

Projects
None yet
5 participants
@cwchien
Contributor

cwchien commented Feb 22, 2017

As #1853 said, media manager still show "File already exists. Nothing done." even the checkbox "Overwrite existing files" is checked.

  • Wiki has been upgraded to 2017-02-19a and lib/scripts/fileuploaderextended.js is new version.
  • Apache server has been restarted and browser cache has been cleaned.

I noticed Line326 of fileuploaderextended.js is old syntax,

params['ow'] = jQuery('.dw__ow').attr('checked');

So I changed it to

params['ow'] = jQuery('.dw__ow').is(':checked');

But still not work.

@Klap-in

This comment has been minimized.

Show comment
Hide comment
@Klap-in

Klap-in Feb 22, 2017

Collaborator

What happens if you re-save the Configuration manager?

Normally Ctrl+F5 is enough in the browser..

Collaborator

Klap-in commented Feb 22, 2017

What happens if you re-save the Configuration manager?

Normally Ctrl+F5 is enough in the browser..

@cwchien

This comment has been minimized.

Show comment
Hide comment
@cwchien

cwchien Feb 22, 2017

Contributor

Thanks for reply.

Just tried re-save the Configuration Manager and Ctrl+F5 in Google Chrome, still not work.

  • CentOS 7.3.1611
  • PHP 5.4.16
  • Apache 2.4.6
Contributor

cwchien commented Feb 22, 2017

Thanks for reply.

Just tried re-save the Configuration Manager and Ctrl+F5 in Google Chrome, still not work.

  • CentOS 7.3.1611
  • PHP 5.4.16
  • Apache 2.4.6
@Klap-in

This comment has been minimized.

Show comment
Hide comment
@Klap-in

Klap-in Feb 22, 2017

Collaborator

Is dokuwiki.org already updated to latest release? It is still presenting the attr() as well.

Collaborator

Klap-in commented Feb 22, 2017

Is dokuwiki.org already updated to latest release? It is still presenting the attr() as well.

@cwchien

This comment has been minimized.

Show comment
Hide comment
@cwchien

cwchien Feb 22, 2017

Contributor

I upgraded my wiki to 2017-02-19a with upgrade plugin.

Contributor

cwchien commented Feb 22, 2017

I upgraded my wiki to 2017-02-19a with upgrade plugin.

@root555

This comment has been minimized.

Show comment
Hide comment
@root555

root555 Feb 23, 2017

I have the same problem on more than one system since the last update installation to DokuWiki Release 2017-02-19a "Frusterick Manners".
The update was done using the DokuWiki Upgrade Plugin and it ended without any errors.
My Environment is Raspbian "jessie" (4.4.38-v7+) with Apache/2.4.10 (Raspbian) and PHP 5.6.29-0+deb8u1

Best regards

root555 commented Feb 23, 2017

I have the same problem on more than one system since the last update installation to DokuWiki Release 2017-02-19a "Frusterick Manners".
The update was done using the DokuWiki Upgrade Plugin and it ended without any errors.
My Environment is Raspbian "jessie" (4.4.38-v7+) with Apache/2.4.10 (Raspbian) and PHP 5.6.29-0+deb8u1

Best regards

@Klap-in

This comment has been minimized.

Show comment
Hide comment
@Klap-in

Klap-in Feb 23, 2017

Collaborator

Do I understand it right that in your case:

  • the file lib/scripts/fileuploaderextended.js is indeed updated in your file system (it contains the fix)
  • but that the javascript presented in the browser is not updated (it does not contain the fix)?
Collaborator

Klap-in commented Feb 23, 2017

Do I understand it right that in your case:

  • the file lib/scripts/fileuploaderextended.js is indeed updated in your file system (it contains the fix)
  • but that the javascript presented in the browser is not updated (it does not contain the fix)?
@KeenRivals

This comment has been minimized.

Show comment
Hide comment
@KeenRivals

KeenRivals Feb 23, 2017

Contributor

I'm having the same problem. If I edit L326 of fileuploaderextended.js as mentioned by OP I still cannot overwrite files.

the file lib/scripts/fileuploaderextended.js is indeed updated in your file system (it contains the fix)
but that the javascript presented in the browser is not updated (it does not contain the fix)?

For me, the file and browser data is updated, but file overwrites still do not work. My server is NGINX on Debian Jessie. I was able to recreate the issue in a fresh install on my desktop (using the MicroApache download option). If I disable Javascript, I can overwrite files as I'd expect.

Contributor

KeenRivals commented Feb 23, 2017

I'm having the same problem. If I edit L326 of fileuploaderextended.js as mentioned by OP I still cannot overwrite files.

the file lib/scripts/fileuploaderextended.js is indeed updated in your file system (it contains the fix)
but that the javascript presented in the browser is not updated (it does not contain the fix)?

For me, the file and browser data is updated, but file overwrites still do not work. My server is NGINX on Debian Jessie. I was able to recreate the issue in a fresh install on my desktop (using the MicroApache download option). If I disable Javascript, I can overwrite files as I'd expect.

@root555

This comment has been minimized.

Show comment
Hide comment
@root555

root555 Feb 23, 2017

My ./lib/scripts/fileuploaderextended.js file on the server has a size of 11677 bytes and the md5 checksum e9c6914f84efbdbe2814aa2044086fb6. It's line 326 is:
params['ow'] = jQuery('.dw__ow').attr('checked');
I'm using Chrome and/or firefox but I chcked it now with Edge too. With all three I'm getting the same result: DokuWiki don't want to overwrite existing files.

root555 commented Feb 23, 2017

My ./lib/scripts/fileuploaderextended.js file on the server has a size of 11677 bytes and the md5 checksum e9c6914f84efbdbe2814aa2044086fb6. It's line 326 is:
params['ow'] = jQuery('.dw__ow').attr('checked');
I'm using Chrome and/or firefox but I chcked it now with Edge too. With all three I'm getting the same result: DokuWiki don't want to overwrite existing files.

@Klap-in

This comment has been minimized.

Show comment
Hide comment
@Klap-in

Klap-in Feb 23, 2017

Collaborator

Now I see what you mean:
.attr('checked'); was at line 245 and at line 326

At line 245 was fixed in Hotfix. Seems logically that fix at line 326 is still required.

Could you test if updating both occurrences to .is(':checked') will fix this?

Collaborator

Klap-in commented Feb 23, 2017

Now I see what you mean:
.attr('checked'); was at line 245 and at line 326

At line 245 was fixed in Hotfix. Seems logically that fix at line 326 is still required.

Could you test if updating both occurrences to .is(':checked') will fix this?

@cwchien

This comment has been minimized.

Show comment
Hide comment
@cwchien

cwchien Feb 24, 2017

Contributor

For my case, both changed to .is(':checked') still not work.

Contributor

cwchien commented Feb 24, 2017

For my case, both changed to .is(':checked') still not work.

@root555

This comment has been minimized.

Show comment
Hide comment
@root555

root555 Feb 24, 2017

I have replaced the "attr" in line 326 by "is" but it didn't changed anything. The problem still persists.

root555 commented Feb 24, 2017

I have replaced the "attr" in line 326 by "is" but it didn't changed anything. The problem still persists.

@bug

This comment has been minimized.

Show comment
Hide comment
@bug

bug Feb 24, 2017

Collaborator

I reproduce it too on a fresh install. The initial fix included in the hotfix release was not enough.

Collaborator

bug commented Feb 24, 2017

I reproduce it too on a fresh install. The initial fix included in the hotfix release was not enough.

@Klap-in

This comment has been minimized.

Show comment
Hide comment
@Klap-in

Klap-in Feb 24, 2017

Collaborator

(I have at the moment no stack for testing, so I keep asking.)

In #1863 it is suggested to fix some php code in lib/media.php:307 as well

Workaround/Bugfix (original line with comments, my line beneath):
lib/media.php:307

$res = media_save(
array('name' => $path,
'mime' => $mime,
'ext' => $ext),
$ns.':'.$id,
//(($INPUT->get->str('ow') == 'checked') ? true : false),
(($INPUT->get->str('ow') == 'true') ? true : false),
$auth,
'copy'

Does this fix combined with the fixes at line 245 and 326 of fileuploaderextended.js help you?

btw: What are steps to reproduce? There are two places to upload files... do you experience this issue at both places in DokuWiki?

  • In the Media Manager (accessible via link at topright of the wiki)
  • and/or in the Media Files pop-up in edit window?
Collaborator

Klap-in commented Feb 24, 2017

(I have at the moment no stack for testing, so I keep asking.)

In #1863 it is suggested to fix some php code in lib/media.php:307 as well

Workaround/Bugfix (original line with comments, my line beneath):
lib/media.php:307

$res = media_save(
array('name' => $path,
'mime' => $mime,
'ext' => $ext),
$ns.':'.$id,
//(($INPUT->get->str('ow') == 'checked') ? true : false),
(($INPUT->get->str('ow') == 'true') ? true : false),
$auth,
'copy'

Does this fix combined with the fixes at line 245 and 326 of fileuploaderextended.js help you?

btw: What are steps to reproduce? There are two places to upload files... do you experience this issue at both places in DokuWiki?

  • In the Media Manager (accessible via link at topright of the wiki)
  • and/or in the Media Files pop-up in edit window?
@root555

This comment has been minimized.

Show comment
Hide comment
@root555

root555 Feb 24, 2017

The problem occurs at both places (in Media Manager and in Media Files pop-up).
Changing both files (media.php, line 307, and fileuploaderextended.js line 326) works for me.
I have done the test on two different installations.

root555 commented Feb 24, 2017

The problem occurs at both places (in Media Manager and in Media Files pop-up).
Changing both files (media.php, line 307, and fileuploaderextended.js line 326) works for me.
I have done the test on two different installations.

@bug

This comment has been minimized.

Show comment
Hide comment
@bug

bug Feb 24, 2017

Collaborator

With both changes, overwriting works for me too:

--- inc/media.php   2017-02-20 20:47:45.154461825 +0100                
+++ inc/media.php   2017-02-24 10:47:30.671367763 +0100                
@@ -304,7 +304,8 @@                                                    
             'mime' => $mime,                                          
             'ext'  => $ext),                                          
         $ns.':'.$id,                                                  
-        (($INPUT->get->str('ow') == 'checked') ? true : false),       
+        (($INPUT->get->str('ow') == 'true') ? true : false),          
         $auth,                                                        
         'copy'                                                        
     );                                                                
--- lib/scripts/fileuploaderextended.js 2017-02-20 20:47:45.181128558 +0100
+++ lib/scripts/fileuploaderextended.js 2017-02-24 10:48:39.578209811 +0100
@@ -323,7 +323,7 @@                                                    
         // build query string                                         
         params = params || {};                                        
         params['qqfile'] = name;                                      
-        params['ow'] = jQuery('.dw__ow').attr('checked');             
+        params['ow'] = jQuery('.dw__ow').is(':checked');              
         var queryString = qq.obj2url(params, this._options.action);   
                                                                       
         xhr.open("POST", queryString, true);                                                                                   
Collaborator

bug commented Feb 24, 2017

With both changes, overwriting works for me too:

--- inc/media.php   2017-02-20 20:47:45.154461825 +0100                
+++ inc/media.php   2017-02-24 10:47:30.671367763 +0100                
@@ -304,7 +304,8 @@                                                    
             'mime' => $mime,                                          
             'ext'  => $ext),                                          
         $ns.':'.$id,                                                  
-        (($INPUT->get->str('ow') == 'checked') ? true : false),       
+        (($INPUT->get->str('ow') == 'true') ? true : false),          
         $auth,                                                        
         'copy'                                                        
     );                                                                
--- lib/scripts/fileuploaderextended.js 2017-02-20 20:47:45.181128558 +0100
+++ lib/scripts/fileuploaderextended.js 2017-02-24 10:48:39.578209811 +0100
@@ -323,7 +323,7 @@                                                    
         // build query string                                         
         params = params || {};                                        
         params['qqfile'] = name;                                      
-        params['ow'] = jQuery('.dw__ow').attr('checked');             
+        params['ow'] = jQuery('.dw__ow').is(':checked');              
         var queryString = qq.obj2url(params, this._options.action);   
                                                                       
         xhr.open("POST", queryString, true);                                                                                   
@Klap-in

This comment has been minimized.

Show comment
Hide comment
@Klap-in

Klap-in Feb 24, 2017

Collaborator

Who can create a pull request with these fixes?

Looking at https://github.com/splitbrain/dokuwiki/search?utf8=%E2%9C%93&q=ow
I got the impression there are no other broken occurrences of usage of the 'ow' url parameter.

The other usage spotted so far is at line 359 of inc/media.php : $INPUT->get->str('ow'). Which is fine I guess. (or 'false' from the .is(':checked') must create a wrong true instead of a false. )
https://github.com/splitbrain/dokuwiki/blob/master/inc/media.php#L359
It would be nice if someone can test this. (no idea how to trigger this function via GUI)

Collaborator

Klap-in commented Feb 24, 2017

Who can create a pull request with these fixes?

Looking at https://github.com/splitbrain/dokuwiki/search?utf8=%E2%9C%93&q=ow
I got the impression there are no other broken occurrences of usage of the 'ow' url parameter.

The other usage spotted so far is at line 359 of inc/media.php : $INPUT->get->str('ow'). Which is fine I guess. (or 'false' from the .is(':checked') must create a wrong true instead of a false. )
https://github.com/splitbrain/dokuwiki/blob/master/inc/media.php#L359
It would be nice if someone can test this. (no idea how to trigger this function via GUI)

@cwchien

This comment has been minimized.

Show comment
Hide comment
@cwchien

cwchien Feb 25, 2017

Contributor

I created the PR for this issue.

BTW, I also confirmed it works.

Contributor

cwchien commented Feb 25, 2017

I created the PR for this issue.

BTW, I also confirmed it works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment