Browser issue #12946

jamran7 · 2016-11-26T21:32:17Z

The solution code is in the commit: Browser issue fix

This is a bug fix for issue:(Brackets turns into a web browser #12441).

Description: Filters out drag/drop events from text strings being dropped into Brackets that will actually change the DOM structure of the Brackets layout. It looks for a “text/url-list” type in the types list and stops the event propagation if there are any.

All unit tests and smoke tests were performed and passed.

Here is a test that verifies that the solution works:
Settings should be as they are in a smoke test

Create an empty folder externally.
Open the folder with Brackets.
Enable "dragDropText" in Brackets preferences.
Drag a text or HTML file into Brackets.
The file should be open and focused in the workspace.
Open up a browser.
Type in any valid URL (a webpage with text) and go there in the browser.
Add a few lines to the test file that was opened in Brackets.
Highlight some text from the webpage and copy it to a line in the test file.
Verify that the text in Brackets can be highlighted and dragged to any other line.
Drag the URL from the Browser’s URL field into Brackets. Do this several times over all areas of the Brackets editor.
Verify that the URL did not drop into Brackets, or Brackets refuses the URL, and that Brackets remains an editor and not a browsable webpage. Failed test: If Brackets does become a webpage restart Brackets and perform steps 1 through 5.
Set the view to Vertical Split.
Open another test file and verify steps 1 through 5 in the other non-used split view.
There should be two test files open, one in each view. Perform steps 6 through 12 for both split editors.
Perform steps 13 through 15 for a Horizontal Split view

My repo: https://github.com/jamran7/brackets

This is on behalf of ChainsawFist

These two sections check for Urls being dragged in to the browser.

So for some reason this changed which I did not perform showed up in my master it seemed that one did not have access and the new one did. In the health status test under extension tests, the connection would timeout.

JSLint want’s this instead of “==“. I had actually done a check with an online checker and I forgot to make the change.

petetnt

Reviewed, nice work ChainsawFist and @jamran7!

If ChainsawFist is the original author as the PR comment points out, I think s/he should also sign the CLA.

petetnt · 2016-11-27T14:44:52Z

src/config.json

@@ -20,7 +20,7 @@
        "extension_url": "https://s3.amazonaws.com/extend.brackets/{0}/{0}-{1}.zip",
        "linting.enabled_by_default": true,
        "build_timestamp": "",
-        "healthDataServerURL": "https://healthdev.brackets.io/healthDataLog"
+        "healthDataServerURL": "https://health.brackets.io/healthDataLog"


Unneeded change here, there's a fix coming for this file getting changed automatically hopefully soon but needs to be removed from this PR. 😕

petetnt · 2016-11-27T14:48:53Z

src/utils/DragAndDrop.js

@@ -156,6 +156,18 @@ define(function (require, exports, module) {
            event = event.originalEvent || event;

            var files = event.dataTransfer.files;
+
+            var types = event.dataTransfer.types;
+            if(!files.length){// stop default behavior if a url is dragged in so the browser does not takeove


Can files also be null? The file handler below seems to suggest that, so it might be worth guarding against it (if(!files || !files.length).

petetnt · 2016-11-27T14:49:12Z

src/utils/DragAndDrop.js

+
+            var types = event.dataTransfer.types;
+            if(!files.length){// stop default behavior if a url is dragged in so the browser does not takeove
+                types.forEach(function (value){


Can types be null?

petetnt · 2016-11-27T14:50:50Z

src/utils/DragAndDrop.js

@@ -174,6 +186,18 @@ define(function (require, exports, module) {
            event = event.originalEvent || event;

            var files = event.dataTransfer.files;
+
+            var types = event.dataTransfer.types;


Might be worth it to move these blocks into a function call such as stopURIListPropagation() or similar instead of repeating the blocks of code.

jamran7 · 2016-11-27T19:16:45Z

ChainsawFist is the organization that I am representing in this PR; it was created as part of a college course project. We have a repo of Brackets and we have been using it to test our bug fixes. I have a cleaner repo of the latest commit, while the one with ChainsawFist is far behind and has multiple changes; we are using the later repo as "show-and-tell" so our professor can keep track of our progress. Right now I'm incorporating fixes that have not been closed by adobe/brackets as part of our assignment. I suppose I could create another repo in ChainsawFist that way there's no confusion and I don't have to keep mentioning the organization. I will make the changes as you suggested. Thank you.

petetnt · 2016-11-27T19:18:28Z

Thanks for the clarification @jamran7!

This reverts commit 887e3d5.

@petetnt

These are the changes as requested by @petetnt. The behavior was packed into a function. Generally `types` is not null but was protected on the case it is. Files must be null or the length must be 0 now.

jamran7 · 2016-11-28T02:34:00Z

Can you rerun that build. api.github.com gave Travis a 403.

Fixed extra indentation and spacing issues

Cleaned up the comments, some were redundant to the functions description. I gave the different cases of `types` values when files and text is dragged/dropped into Brackets.

jamran7 · 2016-11-30T17:27:19Z

I've fixed the indent in the function as well as cleaned up some of the comments.

petetnt · 2016-12-01T14:51:31Z

This looks great, I'll do another round of testing and then merge this in if all is good 👍

petetnt · 2016-12-02T10:11:50Z

Does this PR fix #12961 too? If it doesn't yet, I think it could be trivially added too, right?

jamran7 · 2016-12-05T19:56:18Z

Yes that issue seems to be fixed by this.

petetnt · 2016-12-06T21:18:14Z

Thanks for this great PR @jamran7 & ChainsawFist 👍

jamran7 added 3 commits November 9, 2016 12:32

Browser issue fix

ad3cfed

These two sections check for Urls being dragged in to the browser.

There was a bug in the unit test

887e3d5

So for some reason this changed which I did not perform showed up in my master it seemed that one did not have access and the new one did. In the health status test under extension tests, the connection would timeout.

JSLint syntax fix

9e0538f

JSLint want’s this instead of “==“. I had actually done a check with an online checker and I forgot to make the change.

petetnt suggested changes Nov 27, 2016

View reviewed changes

jamran7 added 2 commits November 27, 2016 18:09

Revert "There was a bug in the unit test"

26fef73

This reverts commit 887e3d5.

Simplify URI-list antipropagation

4d4d85d

These are the changes as requested by @petetnt. The behavior was packed into a function. Generally `types` is not null but was protected on the case it is. Files must be null or the length must be 0 now.

jamran7 added 2 commits November 28, 2016 09:34

Indentation and spacing

9177997

Fixed extra indentation and spacing issues

Clean comments

31356b0

Cleaned up the comments, some were redundant to the functions description. I gave the different cases of `types` values when files and text is dragged/dropped into Brackets.

petetnt self-assigned this Dec 1, 2016

petetnt mentioned this pull request Dec 2, 2016

Dragging an image to window while editing files causes progress loss of unsaved changes #12961

Closed

petetnt approved these changes Dec 6, 2016

View reviewed changes

petetnt merged commit 973cf51 into adobe:master Dec 6, 2016

4O4 mentioned this pull request Feb 25, 2017

Brackets turns into a web browser #12441

Closed

This was referenced Aug 30, 2021

[CLOSED] Browser issue brackets-archive/bracketsIssues#11014

Open

[CLOSED] Dragging an image to window while editing files causes progress loss of unsaved changes brackets-archive/bracketsIssues#11024

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Browser issue #12946

Browser issue #12946

jamran7 commented Nov 26, 2016 •

edited

Loading

petetnt left a comment

petetnt Nov 27, 2016

petetnt Nov 27, 2016

petetnt Nov 27, 2016

petetnt Nov 27, 2016

jamran7 commented Nov 27, 2016

petetnt commented Nov 27, 2016

jamran7 commented Nov 28, 2016

jamran7 commented Nov 30, 2016

petetnt commented Dec 1, 2016

petetnt commented Dec 2, 2016

jamran7 commented Dec 5, 2016

petetnt commented Dec 6, 2016

Browser issue #12946

Browser issue #12946

Conversation

jamran7 commented Nov 26, 2016 • edited Loading

petetnt left a comment

Choose a reason for hiding this comment

petetnt Nov 27, 2016

Choose a reason for hiding this comment

petetnt Nov 27, 2016

Choose a reason for hiding this comment

petetnt Nov 27, 2016

Choose a reason for hiding this comment

petetnt Nov 27, 2016

Choose a reason for hiding this comment

jamran7 commented Nov 27, 2016

petetnt commented Nov 27, 2016

jamran7 commented Nov 28, 2016

jamran7 commented Nov 30, 2016

petetnt commented Dec 1, 2016

petetnt commented Dec 2, 2016

jamran7 commented Dec 5, 2016

petetnt commented Dec 6, 2016

jamran7 commented Nov 26, 2016 •

edited

Loading