Closed Bug 586015 Opened 14 years ago Closed 14 years ago

[IDE] Opening a duplicate file should not open a second or more instances

Categories

(Testing Graveyard :: Mozmill, defect)

defect
Not set
critical

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: aaronmt, Assigned: harth)

Details

(Keywords: dataloss, Whiteboard: [mozmill-1.4.2+])

Attachments

(2 files)

In 1.4.2b3, the IDE will allow one to open a duplicate file more than once and create a new reference to it in the list of open files. We should be smarter here and just switch to the already open file.
Whiteboard: [mozmill-1.4.2?]
Don't think this should be blocking.

I actually disagree with this, except for bug 487588 (we save the test file to disk everytime we run it, could lead to unexpected results if you have two open). Switching to the open file would only be okay if we gave the user an option to open another reference to the same file, like the switch-to-tab functionality in the awesomebar of Firefox.
Whiteboard: [mozmill-1.4.2?] → [mozmill-1.4.2-]
(In reply to comment #1)
> open). Switching to the open file would only be okay if we gave the user an
> option to open another reference to the same file, like the switch-to-tab
> functionality in the awesomebar of Firefox.

We shouldn't allow opening the same file in another tab at all. What Aaron said, instead of creating a new tab with the same test file as content, we should switch to an already open but hidden tab, which contains the to load test file already.

Right now, this bug can cause data loss if we don't watch your steps closely enough.
Severity: normal → critical
Keywords: dataloss
Tabs?

I'm talking about the single drop-down list of open files.
(In reply to comment #2)
> We shouldn't allow opening the same file in another tab at all. What Aaron
> said, instead of creating a new tab with the same test file as content, we
> should switch to an already open but hidden tab, which contains the to load
> test file already.

Why? I can see wanting to try out two different forks of the same test, that's like saying you don't want to have two Gmails open in Firefox, most of the time you don't, but maybe you want to have two different contexts.

> Right now, this bug can cause data loss if we don't watch your steps closely
> enough.

No more data loss than before though.
(In reply to comment #4)
> (In reply to comment #2)
> > We shouldn't allow opening the same file in another tab at all. What Aaron
> > said, instead of creating a new tab with the same test file as content, we
> > should switch to an already open but hidden tab, which contains the to load
> > test file already.
> 
> Why? I can see wanting to try out two different forks of the same test, that's
> like saying you don't want to have two Gmails open in Firefox, most of the time
> you don't, but maybe you want to have two different contexts.
> 
> > Right now, this bug can cause data loss if we don't watch your steps closely
> > enough.
> 
> No more data loss than before though.

Yes but it's an editor and we should duplicate that behaviour that is expected in editors. For example, I am testing TextMate and TextEdit, a single instance of the running application. I open a file 'a.js' it opens. I opt-in again to open a.js, and rather than two, the program will focus on the already present and currently open file.
(In reply to comment #5)
> Yes but it's an editor and we should duplicate that behaviour that is expected
> in editors. For example, I am testing TextMate and TextEdit, a single instance
> of the running application. I open a file 'a.js' it opens. I opt-in again to
> open a.js, and rather than two, the program will focus on the already present
> and currently open file.

I see, good call. I don't know if I would say its blocking, but I could probably get a fix for this before next beta.
Assignee: nobody → fayearthur+bugs
Attachment #465002 - Flags: review?(jhammel)
Comment on attachment 465002 [details] [diff] [review]
switch to file if it is already open

>diff --git a/mozmill/mozmill/extension/content/editor/editor.js b/mozmill/mozmill/extension/content/editor/editor.js
>index f9c9c54..c0c18b7 100644
>--- a/mozmill/mozmill/extension/content/editor/editor.js
>+++ b/mozmill/mozmill/extension/content/editor/editor.js
>@@ -49,6 +49,16 @@ var editor = {
>     this.switchTab();
>   },
> 
>+  switchToFile : function(filename) {
>+    for(var i = 0; i < this.tabs.length; i++) {
>+      if(this.tabs[i].filename == filename) {
>+        this.switchTab(i);
>+        return true;
>+      }
>+    }
>+    return false;
>+  },
>+
>   openNew : function(content, filename) {
>     if(!filename) {
>       this.tempCount++;
>@@ -85,7 +95,7 @@ var editor = {
> 
>   changeFilename : function(filename) {
>     this.currentTab.filename = filename;
>-    this.showFilename(getBasename(filename));	
>+    this.showFilename(getBasename(filename)); 
>   },
> 
>   onFileChanged : function() {
>diff --git a/mozmill/mozmill/extension/content/menus.js b/mozmill/mozmill/extension/content/menus.js
>index 9307995..b0e46f8 100644
>--- a/mozmill/mozmill/extension/content/menus.js
>+++ b/mozmill/mozmill/extension/content/menus.js
>@@ -52,7 +52,8 @@ function openFile(){
>   var openObj = utils.openFile(window);
>   if (openObj) {
>     $("#tabs").tabs("select", 0);
>-    editor.openNew(openObj.data, openObj.path);
>+    if(!editor.switchToFile(openObj.path))
>+      editor.openNew(openObj.data, openObj.path);
>   }
> }
> 

swithToFile conflates an informational function (is the file already open?) with a side-effect (switching the tab).  Is there any way that it can just check what tab it is, maybe returning -1 if it doesn't exist?  Then have the switchTab call in onFileChanged?

Or can you explain to me why it should be this way?
Attachment #465002 - Flags: review?(jhammel) → review-
Comment on attachment 465016 [details] [diff] [review]
seperate switching and checking logic per review comments

looks good
Attachment #465016 - Flags: review?(jhammel) → review+
marking blocking since we have a fix.
Whiteboard: [mozmill-1.4.2-] → [mozmill-1.4.2+]
Verified Fixed, opening a duplicate file will focus on the already open file now and not open a second. Thanks!
Status: RESOLVED → VERIFIED
Product: Testing → Testing Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: