102275 - Number untitled pages

Status: RESOLVED FIXED

(SeaMonkey :: Composer, enhancement)

Description

[TucsonTester2]

Build ID: 20010924

If you open more than one page in composer and do not title them, then you will
have a lot of windows named "untitled - composer" in the task menu.  It would be
a nice enhancemant to have composer number untitled pages to help keep track of
what pages you want to switch to using the task menu.

Reproducible: Always

Steps to Reproduce:
1.Open Composer
2.Click on the new button in the toolbar
3.Click on the Task menu

Actual Results:
The task menu will show two identical menu items labeled "untitled - composer".
 This can create problems if you switch from composer to the browser.  You will
not know which one you want to switch back to.

Expected Results:

I would expect to see something in the menu like:

Untitled - Composer
Untitled 2 - Composer

This would make page switching much easier.

Comment 1

[Keyser Sose]

Marking NEW.

Comment 2

[Kathleen :Brade]

This would be great; cc Ryan in case he has time to work on this.

Comment 3

[Ryan Cassin]

Attachment: Potential fix

Comment 4

[Ryan Cassin]

Reassign to [e-mail], CC: brade.

Brade, I've attached a patch which does exactly what we want... it 
displays "untitled1 - Composer" in the titlebar and in the "Window" menu. 
However, I'm using a global variable in nsEditorShell.cpp (the only global 
variable if I remember correctly) to keep track of the number of untitled 
pages we've opened. Can you think of a better way to do this or is a global 
variable alright?

Thanks for your thoughts!

Comment 5

[Ryan Cassin]

Attachment: Fix version 2

Much improved, no global variable now.

Comment 6

[Ryan Cassin]

I have a fix for this and need the appropriate reviews... realizing of course 
that this is by no means a priority for anyone.

Comment 7

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Comment on attachment 78490 [details] [diff] [review]
Fix version 2

r=glazman providing the fact you fix the typo in GetNextUntitiledValue name.

Btw, thanks for carrying this Ryan.

Comment 8

[Ryan Cassin]

Attachment: Hopefully the last patch

I can spell "Untitled" right, honest.

Comment 9

[Kathleen :Brade]

Comment on attachment 78530 [details] [diff] [review]
Hopefully the last patch

unfortunately editorshell is going away

Ryan--could you try porting this patch into JS?  If you don't have time, feel
free to reassign to me.

Comment 10

[Akkana Peck]

nsbeta1+ since it has a patch that just needs to be ported to the
non-editorshell world.

Comment 11

[Samir Gehani]

Composer triage team: nsbeta1-

Comment 12

[Robert Kaiser]

This bug report is registered in the SeaMonkey product, but has been without a comment since the inception of the SeaMonkey project. This means that it was logged against the old Mozilla suite and we cannot determine that it's still valid for the current SeaMonkey suite. Because of this, we are setting it to an UNCONFIRMED state.

If you can confirm that this report still applies to current SeaMonkey 2.x nightly builds, please set it back to the NEW state along with a comment on how you reproduced it on what Build ID, or if it's an enhancement request, why it's still worth implementing and in what way.
If you can confirm that the report doesn't apply to current SeaMonkey 2.x nightly builds, please set it to the appropriate RESOLVED state (WORKSFORME, INVALID, WONTFIX, or similar).
If no action happens within the next few months, we move this bug report to an EXPIRED state.

Query tag for this change: mass-UNCONFIRM-20090614

Comment 13

[Robert Kaiser]

MASS-CHANGE:
This bug report is registered in the SeaMonkey product, but still has no comment since the inception of the SeaMonkey project 5 years ago.

Because of this, we're resolving the bug as EXPIRED.

If you still can reproduce the bug on SeaMonkey 2 or otherwise think it's still valid, please REOPEN it and if it is a platform or toolkit issue, move it to the according component.

Query tag for this change: EXPIRED-20100420

Comment 14

[Samuel Sidler (:ss)]

KaiRo: I don't think you should have blindly closed bugs that were enhancements.

Comment 15

[Robert Kaiser]

(In reply to comment #14)
> KaiRo: I don't think you should have blindly closed bugs that were
> enhancements.

There was a warning that this will be done more than 10 months ago, and nobody cared about this. Also, this has been discussed extensively on the SeaMonkey newsgroups.

So, this is a bug nobody seemed to care about, and a ton of RFEs are actually just as much dead as many other reports. If nobody triages the bugs he cares about, they die, and that's good that way.

Now that I spent a huge amount of time looking at this bug, I'll move it to NEW, but most others deserve to have died today.

Comment 16

[Edmund Wong (:ewong)]

Attachment: Added numbers to the Untitled title to differentiate pages.

Comment 17

[Edmund Wong (:ewong)]

Comment on attachment 547932 [details] [diff] [review]
Added numbers to the Untitled title to differentiate pages.

With lots of help from Philip Chee.

Comment 18

[[e-mail]]

Comment on attachment 547932 [details] [diff] [review]
Added numbers to the Untitled title to differentiate pages.

>  * ***** END LICENSE BLOCK ***** */
>+Components.utils.import("resource:///modules/editorVar.jsm");
Nit: blank line between please.

>+    title = GetString("untitled") + " " + gWindowNumber;
[I'm not sure this is the best idea. Probably formatting a string would be better, then it would be untitled(1) or untitled-1 or something, but localisers would be able to choose something that works for them. Sadly editor doesn't seem to be a big consumer of formatted strings (and why on earth is it still manually creating string bundles?) Maybe someone will clean up the string bundles at some point making it easier to format the string.]

>+    var titleWindow = GetString("untitled") + " " + gWindowNumber;
Rather than computing this each time, maybe you should create gUntitledString instead of gWindowNumber?

>diff --git a/suite/modules/Makefile.in b/suite/modules/Makefile.in
editor.js is actually shared by Thunderbird so we can't unilaterally require a module that is only available in suite.

>+  editorVar.jsm \
[Need to think up a better name for this. I notice that some of the functions in editorUtilities.js could be put in a jsm instead, so perhaps we could call this editorUtilities.jsm and move other stuff in at a later date?]

>+ * The Original Code is mozilla.org code.
[You're allowed to give this a name, such as editor utility code.]

>+ * The Initial Developer of the Original Code is
>+ * the Mozilla Foundation.
[IIRC this only applies to actual Mozilla employees.]

Comment 19

[Edmund Wong (:ewong)]

Attachment: Added numbers to the Untitled title to differentiate pages. (v2)

Comment 20

[[e-mail]]

Comment on attachment 549652 [details] [diff] [review]
Added numbers to the Untitled title to differentiate pages. (v2)

> ifdef MOZ_SUITE
> EXTRA_COMPONENTS = nsComposerCmdLineHandler.manifest nsComposerCmdLineHandler.js
>+EXTRA_JS_MODULES += \
>+  editorUtilities.jsm \
>+  $(NULL)
The problem is that Thunderbird includes editor.js, and the import will fail if it doesn't have the .jsm file.

>-untitled=untitled
>+untitled=untitled-%S
Need to rename the string.

Comment 21

[[e-mail]]

Comment on attachment 549652 [details] [diff] [review]
Added numbers to the Untitled title to differentiate pages. (v2)

>@@ -2003,17 +2005,19 @@ function UpdateWindowTitle()
>+    var gUntitledString = GetFormattedString("untitled", GetNextUntitledValue());
This isn't going to work because you get the next value every time the window title updates, instead of once per window. (Or maybe just once per untitled window, if you're really smart...)

Comment 22

[Edmund Wong (:ewong)]

Attachment: Added numbers to the Untitled title to differentiate pages. (v3)

Comment 23

[[e-mail]]

Comment on attachment 551299 [details] [diff] [review]
Added numbers to the Untitled title to differentiate pages. (v3)

>+    gWindowNumber = GetNextUntitledValue();
You don't seem to have declared gWindowNumber or gUntitledString.

>   var title = document.title;
>   if (!title)
>-    title = GetString("untitled");
>+    title = GetString("untitledTitle");
If you try to close an untitled document without saving it suggests you might want to save changes to untitled-%S... if you'd saved gUntitledString when you got the next untitled value you could have used that instead.

Also I tried saving an untitled document and after OKing the blank title it prompted me to save the page as "null". Oops.

Comment 24

[Edmund Wong (:ewong)]

Attachment: Added numbers to the Untitled title to differentiate pages. (v4)

Comment 25

[[e-mail]]

Comment on attachment 552924 [details] [diff] [review]
Added numbers to the Untitled title to differentiate pages. (v4)

>-    SetDocumentTitle(TrimString(result.value));
>+  {
>+    if (result.value)
>+      SetDocumentTitle(TrimString(result.value));
>+    else
>+      SetDocumentTitle(gUntitledString);
>+  }
This is definitely wrong. The actual problem is that there's another call to GetString("untitled") that you've overlooked. (At least you managed to find and fix the one for the prompt to save changes all on your own.) Actually MXR tells me there are 5 uses of that string, so I guess they all need fixing...

Comment 26

[Edmund Wong (:ewong)]

Attachment: Added numbers to the Untitled title to differentiate pages. (v5)

Comment 27

[[e-mail]]

Comment on attachment 554082 [details] [diff] [review]
Added numbers to the Untitled title to differentiate pages. (v5)

>@@ -907,17 +907,22 @@ function PromptAndSetTitleIfNone()
>   var promptService = GetPromptService();
>   if (!promptService) return false;
> 
>   var result = {value:null};
>   var captionStr = GetString("DocumentTitle");
>   var msgStr = GetString("NeedDocTitle") + '\n' + GetString("DocTitleHelp");
>   var confirmed = promptService.prompt(window, captionStr, msgStr, result, null, {value:0});
>   if (confirmed)
>-    SetDocumentTitle(TrimString(result.value));
>+  {
>+    if (result.value)
>+      SetDocumentTitle(TrimString(result.value));
>+    else
>+      SetDocumentTitle(gUntitledString);
>+  }
As I said this before, this change is wrong. Please change it back!

>-    title = "("+GetString("untitled")+")";
>+    title = "("+gUntitledString+")";
I'm not actually sure how to get to this screen, but it lives in a dialog, so you have to use opener.gUntitledString instead. Also while you're there can you put spaces around the +s to make it a bit neater?

Comment 28

[Edmund Wong (:ewong)]

Attachment: Added numbers to the Untitled title to differentiate pages. (v6)

Comment 29

[Jens Hatlak (:InvisibleSmiley)]

Hmm, I was about to check this in but AFAIK editor/ is under TB devs control, so I think we need an sr from them just to be sure.

Comment 30

[Ian Neal]

(In reply to Jens Hatlak (:InvisibleSmiley) from comment #29)
> Hmm, I was about to check this in but AFAIK editor/ is under TB devs
> control, so I think we need an sr from them just to be sure.

Yes, it needs at least an r= from the likes of Standard8

Comment 31

[Axel Hecht [:Pike]]

To copy from irc, as it's not 9 untitled windows, but untitled window number 9, we don't need any plural handling here.

Comment 32

[Mark Banner (:standard8)]

Comment on attachment 554689 [details] [diff] [review]
Added numbers to the Untitled title to differentiate pages. (v6)

Review of attachment 554689 [details] [diff] [review]:
-----------------------------------------------------------------

::: editor/ui/locales/en-US/chrome/composer/editor.properties
@@ +109,5 @@
>  Percent=percent
>  PercentOfCell=% of cell
>  PercentOfWindow=% of window
>  PercentOfTable=% of table
> +untitledTitle=untitled-%S

note: You should put a localisation note here explaining what %S is replaced with.

Comment 33

[Edmund Wong (:ewong)]

Attachment: Added numbers to the Untitled title to differentiate pages. (v7)

Added localization note.

Comment 34

[Edmund Wong (:ewong)]

Attachment: Added numbers to the Untitled title to differentiate pages. (v8)

Unbitrotted previous patch.

Comment 35

[Edmund Wong (:ewong)]

Pushed to comm-central : http://hg.mozilla.org/comm-central/rev/a5059b3c07fb