Clicking an Image file in the project tree calls FileOpen command twice

[couzteau]

1. Open a text file
2. Open the chrome dev tools, set break point in DocumentCommangHandlers doOpen.
3. Click on an image in the project tree

result image is opened.

Bug: break point is hit twice. because ProjectManager will run

1. first FileOpen command when the item in the tree is first clicked (ProjectManager._renderTree has an anonymous function to call FileViewController.openAndSelectDocument) and then
2. it will send another FileOpen command when the event to highlight the selection after the image has been opened is sent is received in ProjectManager._documentSelectionFocusChange()

In case of displaying an image the 2nd pass is completely redundant because the image is already in view.

But the 2nd pass is not expected by the Jasmine test runner thus events from the 2nd run may mess op subsequent tests, i.e. tests that are counting events sent on fileOpen.

[peterflynn]

This is less bad now that #6002 has landed: FileViewController still tried to open the same image a second time, but DocumentCommangHandlers is smart enough to no-op

[pthiess]

Made it low priority and assigned it to @couzteau.

[couzteau]

This only occurs when switching from a text file to an image. When going from an image to another image the bug does not ocur.

[couzteau]

I believe the root cause is a race condition. showCustomViewer first clears the an existing text editor by calling DocumentManager._clearCurrentDocument which triggers a documentSelectionFocusChange event picked up by ProjectManager which redraws the project tree. But by the time this event is picked up has already displayed the viewer and made the image file the currentlyViewedFile. Thus the tree gets confused because the first currentDocumentChange event from the _clearCurrentDocument call wipes the original selection of the image file and it also clears the working set selection. The latter is still need to figure out.

[couzteau]

Another way of putting it: the 2nd trigger the comes from EditorManager in showCustomViewer via _setCurrentlyViewedPath is completely wrong. That one should not result in an Open command.

That event is sent after showing the image to update the tree. But tree selection should happen before the first open command in ProjectManager, same as text files. side note: This also work for image files when switching from one image to another.

[couzteau]

posted fix on branch: https://github.com/adobe/brackets/tree/couzteau/fix-6001-imgPreview-dupeOpen

[couzteau]

The fix is not good because removing the clearCurrentDocument call prevents the currentDocumentChange event to be sent, breaking the unit test.

[couzteau]

@peterflynn Time for the code review of the fix?

[dangoor]

I've just confirmed that this will be fixed when the project manager revamp (#8788) lands.

[couzteau]

Cools

[dangoor]

FBNC @couzteau

[couzteau]

Confirming fix. Closing.