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)
Testing Graveyard
Mozmill
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: aaronmt, Assigned: harth)
Details
(Keywords: dataloss, Whiteboard: [mozmill-1.4.2+])
Attachments
(2 files)
1.38 KB,
patch
|
k0scist
:
review-
|
Details | Diff | Splinter Review |
1.41 KB,
patch
|
k0scist
:
review+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Updated•14 years ago
|
Whiteboard: [mozmill-1.4.2?]
Assignee | ||
Comment 1•14 years ago
|
||
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.
Assignee | ||
Updated•14 years ago
|
Whiteboard: [mozmill-1.4.2?] → [mozmill-1.4.2-]
Comment 2•14 years ago
|
||
(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
Reporter | ||
Comment 3•14 years ago
|
||
Tabs? I'm talking about the single drop-down list of open files.
Assignee | ||
Comment 4•14 years ago
|
||
(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.
Reporter | ||
Comment 5•14 years ago
|
||
(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.
Assignee | ||
Comment 6•14 years ago
|
||
(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 | ||
Comment 7•14 years ago
|
||
Assignee: nobody → fayearthur+bugs
Attachment #465002 -
Flags: review?(jhammel)
Comment 8•14 years ago
|
||
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-
Assignee | ||
Comment 9•14 years ago
|
||
Attachment #465016 -
Flags: review?(jhammel)
Comment 10•14 years ago
|
||
Comment on attachment 465016 [details] [diff] [review] seperate switching and checking logic per review comments looks good
Attachment #465016 -
Flags: review?(jhammel) → review+
Assignee | ||
Comment 11•14 years ago
|
||
marking blocking since we have a fix.
Whiteboard: [mozmill-1.4.2-] → [mozmill-1.4.2+]
Assignee | ||
Comment 12•14 years ago
|
||
master: http://github.com/mozautomation/mozmill/commit/a5d1edccca8c3b7cbf0c615fd627e543e3261e8c 1.4.2: http://github.com/mozautomation/mozmill/commit/c18b9145567be40e7bc215b370fd299eada48d5b
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 13•14 years ago
|
||
Verified Fixed, opening a duplicate file will focus on the already open file now and not open a second. Thanks!
Status: RESOLVED → VERIFIED
Updated•8 years ago
|
Product: Testing → Testing Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•