Skip to content
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

hash in url in files app leads to root #14828

Closed
dttpy opened this issue Mar 12, 2015 · 16 comments · Fixed by #19418
Closed

hash in url in files app leads to root #14828

dttpy opened this issue Mar 12, 2015 · 16 comments · Fixed by #19418

Comments

@dttpy
Copy link

dttpy commented Mar 12, 2015

Steps to reproduce

  1. create a folder "pics"
  2. upload a picture
  3. click and open this picture
  4. close this picture
  5. refresh the page

Expected behaviour

The page stays at the folder "pics", and the url should be

http://.../owncloud802/index.php/apps/files/?dir=%2Fpics

Actual behaviour

It goes back to the main page with the url:

http://.../owncloud802/index.php/apps/files/?dir=%2Fpic2#/pics/P100.JPG

Server configuration

Operating system: Ubuntu 13.10

Web server: apache 2.4.6 (Ubuntu)

Database: SQLite

PHP version: 5.5.3-1ubuntu2.3

ownCloud version: 8.0.2

Updated from an older ownCloud or fresh install: 8.0.0-beta2

List of activated apps: standard apps of owncloud

The content of config/config.php:

<?php
define('DEBUG',true);
$CONFIG = array (
  'instanceid' => 'oc6d9cda6e26',

  'datadirectory' => '/var/ocdata/.../owncloud/data8',
  'dbtype' => 'sqlite3',
  'version' => '8.0.2.0',
  'installed' => true,
  'theme' => '',
  'maintenance' => false,
  'maxZipInputSize' => 838860800,
  'allowZipDownload' => true,
  'loglevel' => '0',
  'trusted_domains' => 
  array (
    0 => '192.168.1.80',
  ),
  'appcodechecker' => false,


  );



Are you using external storage, if yes which one: no
Are you using encryption: yes

Client configuration

Browser: firefox 35.0.1

Operating system: ubuntu 13.10

Logs

Web server error log

ownCloud log (data/owncloud.log)

Browser log

@dttpy
Copy link
Author

dttpy commented Mar 12, 2015

@PVince81

@DeepDiver1975
Copy link
Member

@LukasReschke encoding fun? 🙊

@PVince81
Copy link
Contributor

I don't think so.
The strange thing is that if you remove everything behind the "#" and keep the hash, it works.

I suspect that the URL parsing code is parsing too much.

@DeepDiver1975 DeepDiver1975 added this to the 8.1-current milestone Mar 12, 2015
@dttpy
Copy link
Author

dttpy commented Mar 13, 2015

@PVince81 maybe this can help owncloud/gallery#50

@PVince81
Copy link
Contributor

Even if the gallery app properly cleans up the URL, we still need to make sure to fix the parsing code

@rullzer
Copy link
Contributor

rullzer commented Apr 20, 2015

So it seems the files app is just not expecting any # in the url.
The magic happens at https://github.com/owncloud/core/blob/master/apps/files/js/app.js#L49 which calls https://github.com/owncloud/core/blob/master/core/js/js.js#L1595 which calls https://github.com/owncloud/core/blob/master/core/js/js.js#L1572

The # breaks the url because of the last function. Since that looks for the location of the # in the url. Normally this works because there is no # and all can be parsed as it should. But once there is a # only the last part gets parsed. And of course then it does not find valid stuff.

This means the following URL will work:
.....?#dir=

So it seems the files app just abuses some functions that most of the time return the correct result.

@PVince81
Copy link
Contributor

@rullzer the hash strategy is used as a workaround for IE8 that doesn't support changing the URL.
You might have noticed that when you navigate into folders, only the URL changes. There is no full page reload. IE8 doesn't support that, so we have to abuse the hash part to be able to store the modified URL parts. This also means that the code in question needs to be able to parse URLs generated by IE8. Imagine if someone who has IE8 copies the URL from the browser and pastes it into Firefox (for whatever reasons), the URL still needs to work.

@rullzer
Copy link
Contributor

rullzer commented Apr 20, 2015

@PVince81 aaah good old ie8.
Anyways, I just pointed out where it goes wrong and why. We could simply wait for IE8 support to be dropper. Maybe that is the easiest solution ;)

@PVince81
Copy link
Contributor

I think if we'd remove the IE8 workaround, then it means that IE8 would be back to OC 5 behavior: refresh the whole browser page when navigating. IE8 users are probably already used to sub-par user experience 😉

@oparoz
Copy link
Contributor

oparoz commented Apr 21, 2015

While the parseHashQuery() method still needs to be fixed, the specific problem described in the OP has been fixed in Gallery+, in master for now.

@PVince81
Copy link
Contributor

Setting severity to low as there's an easy workaround

@ghost ghost modified the milestones: 8.2-next, 8.1-current May 11, 2015
@oparoz
Copy link
Contributor

oparoz commented Jun 30, 2015

Coming back to this as it's breaking the Files app when users use the browser's forward button.
With lots of workarounds it's possible to make the slideshow not break when the back button is hit, but we have no control on the URL the user has just left and if he ever presses forward, the app is reset.

@PVince81
Copy link
Contributor

Yes... maybe we need a way for apps to define URL routes in JS so we could reroute requests to them.
And instead of using hashes hacks, use real actual URLs for file viewers. (only IE8 would not approve and require special workarounds)

@PVince81
Copy link
Contributor

Ok, actually the file list should make sure that onPopstate doesn't trigger a folder change event if it's the same folder. That might help fixing it.

@PVince81
Copy link
Contributor

Similar issue: #19120 (comment)

@PVince81
Copy link
Contributor

Fix is here: #19418

Now also reproducible with the next text editor, when closing the editor then refreshing the page.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants