If there is no browser history, exit the slideshow by calling stop.

[setnes]

This is a fix for #460.

This is the same code that was in #463.

I am renaming branches. Sorry for any confusion. :)

[oparoz]

This doesn't seem to work for me, but maybe it's a corner case...

1. Share a folder containing 1 JPEG
2. Go to the link
3. Click on the image
4. Paste that link in another browser
5. Click close

Both with and without the patch, I end up with an empty tab

[setnes]

I think I need to explain this. I'm trying to fix something very similar, but not exactly what you are testing.

1. Your browser history is not empty in your test. It's subtle, but your first history entry is actually your browser start page.
2. Your test case actually works the way I think it should work. You went back a page in your history when you clicked the X. To me this seems natural.
3. What I am fixing is when there is NO history... like when you click on a link in an e-mail and a new browser window or tab is opened. When there is NO history, clicking the X used to do nothing. You were essentially stuck in the page. Now it closes the slideshow image and you are back in a gallery.

I don't think you can actually read history entries. That's by design. They don't want you to know what other pages the user has been to.

[oparoz]

2. ...To me this seems natural.

To me it doesn't :). As a user, if I'm given a link and there is a X. I should be able to click on it and get the photowall or files list view.
I reckon it's going to be difficult to do without rewriting all the logic, because clicking on the back button should go to the empty page and not just exit the slideshow.

[setnes]

I don't think we need to rewrite everything. But I think we have two different issues. This pull very much fixes one of the two issues and leaves the other one unchanged.

Maybe I'll think about this further and see if there is a way to make this work the way you want it without a big rewrite. One thought I had was to set a var once we have visited any album page and test that along with the history. If it is unset, we may have entered from a direct photo link.

[oparoz]

The problem with the var is that the photowall is loaded in the background, so it will be set as soon as you open the slideshow.

[setnes]

True... but now that we are writing click handlers for navigation, we can set something to know the user entered the slideshow from the gallery or not before navigating. :)

slideshowOpenedFromAlbum = true;

(yeah... probably need a shorter name)

[oparoz]

probably need a shorter name

Yeah, this is not Java :D

But OK, so if clicked, then use history, else just close the slideshow.

[setnes]

What about from the files page instead of a gallery album page? Will we be able to set something in that case?

I actually think there might be an easier fix for this. It's very important that navigating back calls stop(). It's not as important for _exit to call window.history.back though. It could just as easily call stop() instead.

[oparoz]

You're welcomed to give it a go. All I remember is that it took me days to find a working solutions. There was always something which would break under a certain condition. Maybe a variable is the simple solution which always works :)

The specs are still here, so it should help with the debugging.

One thing to keep in mind is that on 8.2+ things have changed in the Files app, using # doesn't reset the app any more.

[setnes]

I worked on this a little, and now agree that a variable is going to be easier. :) This will require code changes that are being made in #471. Should I just add it to that change, or should this wait for it?

Some notes for when I can work on this again...

• You cannot successfully start the slideshow from an image link that uses an "apps/files" path. Right now, we only need to worry about image links that use an "apps/gallery" path.
• The Gallery object exists only when the "apps/gallery" path is used. When opening the slideshow from the files view, the Gallery object is not defined. (this will be helpful)
• We can create a Boolean named Gallery.imageClicked with a default value of false.
• In the new _openImage function from Click handlers for image and album thumbnails. #471, we can set Gallery.imageClicked to true.
• In the _exit function, we should only call window.history.back() IF...
  • We can manipulate history.
  • We have more than 1 history entry.
  • Gallery is not defined OR ( Gallery is defined AND imageClicked is true )
• Otherwise we call this.stop() to close the slideshow and get to a gallery view.

[oparoz]

Let's finish #471 before this one then.

[oparoz]

You can now create a PR against the next-9.1 branch if you want to try and fix this.

[setnes]

I do want to fix this and will give it a shot. It won't happen this weekend though. I'm still here though. :)

[oparoz]

Cool :). I'll release a new version of Gallery+ for 8.2 in the coming weeks, but there will be another one in March.