Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Issue #9934 - error on zip file opening #9971

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

hzahoori
Copy link

Hi @JeffryBooher Please merge, my update to the bug, Now when you drag a file from a zip file it will return an error.
I would love to get your advice on best practices for next bug.Thanks

@@ -486,7 +486,7 @@ define(function (require, exports, module) {
FileSystem.prototype._normalizePath = function (path, isDirectory) {

if (!FileSystem.isAbsolutePath(path)) {
throw new Error("Paths must be absolute: '" + path + "'"); // expect only absolute paths
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than change this to return false, can we add a try/catch block around the drop code to catch this error?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean in the main brackets.js or inside the DragandDrop.js?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually think it would be even better to directly detect the special ZIP path in the DragAndDrop code, and terminate instead of passing it into FileSystem. If we add a try/catch, we run the risk of silently swallowing other errors that would make it difficult to detect & debug other bugs in the future.

That said, I can't reproduce #9934 -- when I drag a file from inside a ZIP over top of Brackets (using the 1.0 shell on Win 7) I just get the "no drop" icon. When I let go, nothing happens (no errors are logged to the console of anything). @JeffryBooher @hzahoori are you both actually seeing a console error in this case? If so, which version of Windows are you on? Are you opening the ZIP file in Windows Explorer, or in some other app like WinZip? And what is the bad path that Brackets is getting?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got some more info from @hzahoori -- this occurs with WinRAR, not with the native Windows Explorer zip viewer. DragAndDrop.openDroppedFiles() gets passed a relative path (relative to the root of the zip file it seems). So we'll probably need to do the check there, just before FileSystem.resolve() gets called.

Probably this means there's a bug on the native side somewhere, either in ClientHandler::OnDragEnter() or in the CefDragData that is consumes, where we're treating something as a file-flavored drop item when it's really something else. But we don't really need to debug into that to fix this....

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a good Idea I believe @peterflynn , although I already pushed changes to take care of this issue, but I would love to get more info on it, what you mean by native.
But what I see is, that when I drag a file from a winrar zip, it shows me that its drop-able, but when I drag a zip file from window explorer it does not show that you can drop it it just shows a big brackets icon with my mouse cursor, and when you leave it inside the editor nothing happens; but this fix, will take care of anything that does not have absolute path, because when I drop a file from a winrar, it does not give me an absolute path, but instead it gives me file/filename.zip, which is not acceptable by brackets.

@hzahoori
Copy link
Author

Hi @peterflynn what next step should be taken for this bug?, is it going to be merged or you are thinking to take care of it in the native? if so, please let me know how you want to deal with it, because if I can I want to look into the in the native aswell?

@nethip
Copy link
Contributor

nethip commented Apr 3, 2015

@peterflynn @hzahoori I had a quick look at ClientHandler::OnDragEnter and there is no code to check for supporting the right file type. I think the change should be do check for all the supported formats.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants