Fix opening a section again in the Files app #11967
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When a section is open in the Files app a "show" event is triggered. File list objects handle that event by reloading themselves, but only if the file list was shown at least once. However, the file list objects of plugins are created when the "show" event is triggered for the first time for their section; as the file list objects register their handler for the "show" event when they are created they never handle the first triggered "show" event, as the handler is set while that event is being already handled. Therefore, from the point of view of the handler, the second time that a "show" event was triggered it was seen as if the file list was shown for the first time, and thus it was not reloaded. Now the "shown" property is explicitly set for those file lists that are created while handling a "show" event, which causes them to be reloaded as expected when opening their section again. Note that it is not possible to just reload the file list whenever it is shown; the file list is reloaded also when the directory changes, and this can happen when the web page is initially loaded and the URL is parsed. In that case, if file lists were reloaded when shown for the first time then it could be reloaded twice, one with the default parameters due to the "show" event and another one with the proper parameters once the URL was parsed, and the files that appeard in the list would depend on which response from the server was received the last. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
The custom handler for "URL changed" events were added to reload the file list whenever the sections for favorites and shares were opened; this was used to fix the problem of not reloading the file lists when opening them for a second time. However, besides that the handlers were not really necessary, and as the root of the bug was fixed in the previous commit those handlers are now removed. The file list for tags uses the handler for a different purpose, though, so that one was kept. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
danxuliu
added
bug
3. to review
❤️ |
|
CI failure is a false positive ;) |
|
@danxuliu Could you open the stable14 backport PR? |
rullzer
approved these changes
Oct 23, 2018
This was referenced Oct 23, 2018
Done. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #11907
When a section is open in the Files app a
showevent is triggered. File list objects handle that event by reloading themselves, but only if the file list was shown at least once. This is needed to prevent a race condition when the Files app is initially loaded, as in that case the file list could be reloaded twice, one time due to theshowevent and another time due to a directory change when parsing the URL.However, the file list objects of plugins are created when the
showevent is triggered for the first time for their section; as the file list objects register their handler for theshowevent when they are created they never handle the first triggeredshowevent, as the handler is set while that event is being already handled. Therefore, from the point of view of the handler, the second time that ashowevent was triggered it was seen as if the file list was shown for the first time, and thus it was not reloaded. Now theshownproperty is explicitly set for those file lists that are created while handling ashowevent, which causes them to be reloaded as expected when opening their section again."But, wait, before this fix the file list for favorites and shares did not have the
shownproperty set, yet they were reloaded without problems the second time that their section was opened, why?". Because they overrode theonUrlChangedmethod to change the directory; as the URL changes when a different section is opened (due to the trailing view=XXX part) the file list always got reloaded, although not necessarily due to theshowevent. The method was overrode for both favorites and shares to fix the file list not being reloaded when showing it again, so this pull request removes them as now that theshownproperty is explicitly set that fix is no longer needed.In the case of the file list for system tags, however, the
_onUrlChangedmethod was not removed as it was overrode for a different purpose (parsing the tags to search for from the URL). Note that opening again that section clears the tags to search for previously set, but that is a different issue than the one fixed here (and it is not too bad either, as searching for the tags again works without problems).