Last Comment Bug 705234 - Inconsistent use of "full screen" and "full-screen" across browser and DOM strings, should use "fullscreen" instead
: Inconsistent use of "full screen" and "full-screen" across browser and DOM st...
Status: ASSIGNED
:
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Abhishek Bhatnagar
:
Mentors:
Depends on: 716107
Blocks: 545812
  Show dependency treegraph
 
Reported: 2011-11-25 00:17 PST by Francesco Lodolo [:flod] - OFFLINE Jun 26-Jul 3
Modified: 2015-03-26 05:57 PDT (History)
18 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Proposed fix for bug 705234/ (11.88 KB, patch)
2012-03-14 16:20 PDT, Abhishek Bhatnagar
no flags Details | Diff | Review
Proposed fix for bug 705234 - reformatted (16.94 KB, patch)
2012-03-15 09:30 PDT, Abhishek Bhatnagar
jaws: feedback+
Details | Diff | Review
Bug 705234 - Corrected inconsistent use of "full-screen", "full screen" and "Full-screen" (509.45 KB, patch)
2012-04-09 19:41 PDT, Abhishek Bhatnagar
jaws: feedback+
Details | Diff | Review
Screenshots of inconsistant use of Fullscreen (64.69 KB, image/png)
2015-03-25 16:21 PDT, toe_head2001
no flags Details

Description Francesco Lodolo [:flod] - OFFLINE Jun 26-Jul 3 2011-11-25 00:17:01 PST
Old strings in /browser use "Full screen", new strings in DOM (and one in browser.dtd) use "full-screen". I think this is worth fixing before it spreads like bug 685302
Comment 1 :Margaret Leibovic 2011-11-27 14:38:25 PST
Chris, does the full screen spec specify which string is correct to use?

(I'm also cc'ing Matej, our wordsmith, so that press/marketing can also be in the loop about which way we do this.)
Comment 2 Chris Pearce (:cpearce) 2011-11-27 14:56:25 PST
*sigh*. Browser/F11 full-screen mode may have used "fullscreen"? Our original spec for the DOM API used "full-screen". The editor of the W3 draft spec copied our spec but changed to using "fullscreen" without consulting us...
Comment 3 Matej Novak [:matej] 2011-11-28 08:43:28 PST
(In reply to flod (Francesco Lodolo) from comment #0)
> Old strings in /browser use "Full screen", new strings in DOM (and one in
> browser.dtd) use "full-screen".

Both *could* be correct, but it depends on use and context. Generally and on its own it should be "full screen," but if it's modify something, it needs the hyphen (i.e. "full-screen mode").

(In reply to Chris Pearce (:cpearce) (Mozilla Corporation) from comment #2)
> *sigh*. Browser/F11 full-screen mode may have used "fullscreen"? Our
> original spec for the DOM API used "full-screen". The editor of the W3 draft
> spec copied our spec but changed to using "fullscreen" without consulting
> us...

As far as I know, we don't have a rule about this currently, but if we use "fullscreen" we can be consistent everywhere and not have to worry about the hyphen. That's what I would recommend going forward.
Comment 4 Dão Gottwald [:dao] 2011-11-28 11:17:46 PST
(In reply to Chris Pearce (:cpearce) (Mozilla Corporation) from comment #2)
> *sigh*. Browser/F11 full-screen mode may have used "fullscreen"?

Nope, and we shouldn't switch to "fullscreen" unless that's more common, easier to read, or something like that.

I believe there are a few places where "mode" needs to be added to "full-screen", and a few others that are just missing the hyphen.
Comment 5 Matej Novak [:matej] 2011-11-28 11:26:48 PST
(In reply to Dão Gottwald [:dao] from comment #4)
> (In reply to Chris Pearce (:cpearce) (Mozilla Corporation) from comment #2)
> > *sigh*. Browser/F11 full-screen mode may have used "fullscreen"?
> 
> Nope, and we shouldn't switch to "fullscreen" unless that's more common,
> easier to read, or something like that.

I don't know if it's easier to read, but it is more consistent and less ambiguous. It means we can say things like "go fullscreen" or "go to fullscreen mode" (examples only) without having to worry about whether or not we need a comma. It's what I would strongly recommend going forward.
Comment 6 Dão Gottwald [:dao] 2011-11-28 11:50:18 PST
"go fullscreen" sounds colloquial to me, so I'm not sure how relevant it is here. Looking at our strings, I think we just want "full-screen mode" most of the time. There are strings saying "Exited full-screen", which seems just wrong to me regardless of the hyphen.
Comment 7 Matej Novak [:matej] 2011-11-28 12:23:58 PST
(In reply to Dão Gottwald [:dao] from comment #6)
> Looking at our strings, I think we just want "full-screen mode" most
> of the time.

One major example of where this isn't the case is under the View menu where it says "Full Screen" (which is correct under current usage).

I still don't see anything here that's a blocker for using "fullscreen." It will just make it easier for future instances of the term so no one has to wonder whether it should be hyphenated or not.
Comment 8 Dão Gottwald [:dao] 2011-11-28 12:27:53 PST
Switching to fullscreen because it's easier to write just seems bogus to me. We can handle the extra work load of getting hyphens right. (If we want fullscreen for other reasons, fine by me!)
Comment 9 Matej Novak [:matej] 2011-11-28 13:19:55 PST
When we created the Brand Toolkit, I forgot to create an entry for "fullscreen." Now that I think about it, though, I much prefer it as one word and would like to use that as our style going forward. The fact that it's easier is just an added bonus. Mostly I think it's clear and unambiguous.
Comment 10 Dão Gottwald [:dao] 2011-12-07 00:02:20 PST
So, youtube's FS button is labeled "Full screen" and the Flash FS warning says "exit full screen mode" (not actually wrong; the hyphen isn't mandatory). These are probably the most prominent places where people would encounter this term today.
Comment 11 John Slater 2011-12-08 15:54:14 PST
Jumping in here for a sec...

I've done a little searching around and it seems like there's not an agreed upon standard for fullscreen or full screen or full-screen or whatever...for every example of one you can find an equally relevant example of another.

That said, Matej is our copywriter and he's responsible for how we express ourselves in the written word. So, unless there's a  compelling reason to overrule him (and, as noted, I don't think there is) I think we should just go with his option. Otherwise this debate could go on pretty much endlessly, and I'm sure we all have better things to do.
Comment 12 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-12-09 09:58:26 PST
Let's just use "fullscreen" consistently, as Matej suggests. We can fix up the DOM and Mobile strings while we're at it. (I think internal consistency is far more important than external consistency - there's really no chance of confusion here.)
Comment 13 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2012-01-09 16:58:57 PST
Since I'm adding/changing strings in bug 714172, I'm bringing up the question of platform consistency. Apple uses "Enter Full Screen" and "Exit Full Screen" across apps on OS X Lion. The change in the bug is a departure from our current behavior of a checkbox menu item: "[] Full Screen".
Comment 14 Matej Novak [:matej] 2012-01-10 14:26:48 PST
(In reply to Paul O'Shannessy [:zpao] from comment #13)
> Since I'm adding/changing strings in bug 714172, I'm bringing up the
> question of platform consistency. Apple uses "Enter Full Screen" and "Exit
> Full Screen" across apps on OS X Lion. The change in the bug is a departure
> from our current behavior of a checkbox menu item: "[] Full Screen".

The key point of differentiation is the word "enter," though even there I think using "fullscreen" would be correct. Do we do anything like that in the browser (i.e. have a word in front of "full screen" with nothing following it)?
Comment 15 Dão Gottwald [:dao] 2012-01-10 14:29:57 PST
(In reply to Matej Novak [:matej] from comment #14)
> Do we do anything like that in
> the browser (i.e. have a word in front of "full screen" with nothing
> following it)?

Some console messages have this, I think, but otherwise I tried to make sure this wouldn't happen...
Comment 16 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2012-01-10 17:03:39 PST
(In reply to Matej Novak [:matej] from comment #14)
> Do we do anything like that in
> the browser (i.e. have a word in front of "full screen" with nothing
> following it)?

We have "Exit Full Screen" for the video widget: https://mxr.mozilla.org/mozilla-central/source/toolkit/locales/en-US/chrome/global/videocontrols.dtd#6 (though it looks like it's just for accessibility). The enter text is just "Full Screen".

Other than that, it doesn't look like it.
Comment 17 Abhishek Bhatnagar 2012-01-18 20:17:56 PST
Hey guys, I'd like to take this bug on if no one else has it in their sights. Could it be assigned to me?
Comment 18 Abhishek Bhatnagar 2012-03-14 16:20:01 PDT
Created attachment 606015 [details] [diff] [review]
Proposed fix for bug 705234/

All instances of 'Full Screen' have been left intact as discussed in the bug comments. However 'full screen', 'full-screen' and 'Full-screen' have been upturned to 'fullscreen'.
Comment 19 (Away 6/25-7/4) Jared Wein [:jaws] (reviews and needinfo disabled until back) 2012-03-14 16:35:20 PDT
(In reply to Abhishek Bhatnagar from comment #18)
> Created attachment 606015 [details] [diff] [review]

Can you please reformat your patch based on the steps provided here? https://developer.mozilla.org/en/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F
Comment 20 Abhishek Bhatnagar 2012-03-15 09:30:42 PDT
Created attachment 606251 [details] [diff] [review]
Proposed fix for bug 705234 - reformatted

Corrects inconsistent use of "full-screen", "full screen", "Full-screen" and "Full-Screen". It leaves "Full Screen" as is as per bug comments.
Comment 21 (Away 6/25-7/4) Jared Wein [:jaws] (reviews and needinfo disabled until back) 2012-03-15 13:37:00 PDT
Comment on attachment 606251 [details] [diff] [review]
Proposed fix for bug 705234 - reformatted

Abhishek: Thanks for the patch! When requesting review, please choose a person from the submodule owners & peers list at https://wiki.mozilla.org/Modules to make sure that it gets on someones radar.

I'll do a preliminary review on this and then forward it to a browser peer when it's ready for checking in.
Comment 22 Abhishek Bhatnagar 2012-03-15 13:57:38 PDT
(In reply to Jared Wein [:jaws] from comment #21)
> Comment on attachment 606251 [details] [diff] [review]
> Proposed fix for bug 705234 - reformatted
> 
> Abhishek: Thanks for the patch! When requesting review, please choose a
> person from the submodule owners & peers list at
> https://wiki.mozilla.org/Modules to make sure that it gets on someones radar.
> 
> I'll do a preliminary review on this and then forward it to a browser peer
> when it's ready for checking in.

Ah thanks, I couldn't figure out where to look for module owners.

Will do next time.
Comment 23 (Away 6/25-7/4) Jared Wein [:jaws] (reviews and needinfo disabled until back) 2012-03-22 10:57:29 PDT
Comment on attachment 606251 [details] [diff] [review]
Proposed fix for bug 705234 - reformatted

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

::: browser/locales/en-US/chrome/browser/browser.dtd
@@ +89,5 @@
>  <!ENTITY fullScreenAutohide.label "Hide Toolbars">
>  <!ENTITY fullScreenAutohide.accesskey "H">
>  <!ENTITY fullScreenExit.label "Exit Full Screen Mode">
>  <!ENTITY fullScreenExit.accesskey "F">
> +<!ENTITY domFullScreenWarning.label "Press ESC to leave fullscreen mode">

The entity name should be changed here to let localizers know that the label has changed. Please change the entity name to domFullscreenWarning2.label and update any place that was using the old entity name to use the new entity name.

@@ +509,5 @@
>  <!ENTITY cutButton.tooltip              "Cut">
>  <!ENTITY copyButton.tooltip             "Copy">
>  <!ENTITY pasteButton.tooltip            "Paste">
>  
> +<!ENTITY fullScreenButton.tooltip       "Display the window in fullscreen">

This entity name should be changed too. I recommend fullscreenButton2.tooltip.

::: content/base/src/nsDocument.cpp
@@ +8595,5 @@
>      // Not in full-screen mode.
>      return;
>    }
>    NS_ASSERTION(root->IsFullScreenDoc(),
> +    "Full Screen root should be a fullscreen doc...");

Changing assertion text will cause hg logs to have a somewhat misleading history, but it is good to have the same strings throughout our code so when they inevitably get copy-pasted the correct form gets propagated.

Can you please change this to say:
> "Fullscreen root should be a fullscreen doc..."

@@ +8815,5 @@
>    PRUint32 last = mFullScreenStack.Length() - 1;
>    nsCOMPtr<Element> element(do_QueryReferent(mFullScreenStack[last]));
> +  NS_ASSERTION(element, "Should have fullscreen element!");
> +  NS_ASSERTION(element->IsInDoc(), "Full Screen element should be in doc");
> +  NS_ASSERTION(element->OwnerDoc() == this, "Full Screen element should be in this doc");

Fullscreen instead of Full Screen in the above assertions.

@@ +8929,5 @@
>    // Set the full-screen element. This sets the full-screen style on the
>    // element, and the full-screen-ancestor styles on ancestors of the element
>    // in this document.
>    DebugOnly<bool> x = FullScreenStackPush(aElement);
> +  NS_ASSERTION(x, "Full Screen state of requesting doc should always change!");

Fullscreen here too.

@@ +8967,5 @@
>  #ifdef DEBUG
>    // Note assertions must run before SetWindowFullScreen() as that does
>    // synchronous event dispatch which can run script which exits full-screen!
>    NS_ASSERTION(GetFullScreenElement() == aElement,
> +               "Full Screen element should be the requested element!");

Fullscreen here.

::: dom/locales/en-US/chrome/dom/dom.properties
@@ +127,5 @@
> +FullScreenDeniedSubDocFullScreen=Request for fullscreen was denied because a subdocument of the document requesting fullscreen is already fullscreen.
> +FullScreenDeniedNotDescendant=Request for fullscreen was denied because requesting element is not a descendant of the current fullscreen element.
> +FullScreenDeniedNotFocusedTab=Request for fullscreen was denied because requesting element is not in the currently focused tab.
> +RemovedFullScreenElement=Exited fullscreen because fullscreen element was removed from document.
> +FocusedWindowedPluginWhileFullScreen=Exited fullscreen because windowed plugin was focused.

All of these names should be changed so localizers will know to update their text. I recommend lowercasing the 's' on 'Fullscreen' and placing a 2 at the end of the name.

::: mobile/android/locales/en-US/chrome/browser.properties
@@ +16,5 @@
>  alertDownloadsSize=Download too big
>  alertDownloadsNoSpace=Not enough storage space
>  alertDownloadsToast=Download started…
>  
> +alertFullScreenToast=Press BACK to leave fullscreen mode

alertFullscreenToast2
Comment 24 Robert Kaiser (not working on stability any more) 2012-03-22 11:01:45 PDT
(In reply to Jared Wein [:jaws] from comment #23)
> The entity name should be changed here to let localizers know that the label
> has changed. Please change the entity name to domFullscreenWarning2.label
> and update any place that was using the old entity name to use the new
> entity name.

I disagree here. The entity name does not need to be changed if the change does not require action from localizers. For anyone who actually translated this to a different language, there is no need to act on this change at all, as they need to respect *their* spelling anyhow, and this is only a spelling fix for US English.
Comment 25 Dão Gottwald [:dao] 2012-03-22 12:26:02 PDT
Yes, these are English-specific spelling fixes; don't change the entity names.
Comment 26 Abhishek Bhatnagar 2012-04-09 19:41:08 PDT
Created attachment 613473 [details] [diff] [review]
Bug 705234 - Corrected inconsistent use of "full-screen", "full screen" and "Full-screen"

Does not update Entity names as per discussion
Comment 27 Abhishek Bhatnagar 2012-04-09 19:42:56 PDT
So assuming that entity names did not have to be updated, I made the other updates.

As a note, the following occurrences of "Full Screen" were left intact, because of their context.
(these are grep outputs, hence the format)

browser/locales/en-US/chrome/browser/browser.dtd:80:<!ENTITY fullScreenCmd.label "Full Screen">
browser/locales/en-US/chrome/browser/browser.dtd:91:<!ENTITY fullScreenExit.label "Exit Full Screen Mode">
browser/locales/en-US/chrome/browser/browser.dtd:466:<!ENTITY videoFullScreen.label       "Full Screen">
mobile/android/locales/en-US/chrome/browser.properties:206:contextmenu.fullScreen=Full Screen
mobile/android/app/recommended-addons.json:1:{"addons":[{"id":"fullscreen@mbrubeck.limpet.net","name":"Full Screen","version":"3.2","iconURL":"http://static-cdn.addons.mozilla.net/en-US/firefox/images/addon_icon/252573-32.png?modified=1310547725","homepageURL":"https://addons.mozilla.org/en-US/android/addon/full-screen-252573/?src=api"},{"id":"cloudviewer@starkravingfinkle.org","name":"Cloud Viewer","version":"2.1","iconURL":"https://static-ssl-cdn.addons.mozilla.net/img/uploads/addon_icons/295/295895-32.png?modified=1333044963","homepageURL":"https://addons.mozilla.org/en-US/android/addon/cloud-viewer/?src=api"}]}
mobile/xul/locales/en-US/chrome/browser.dtd:108:<!ENTITY contextFullScreen.label      "Full Screen">
toolkit/locales/en-US/chrome/global/videocontrols.dtd:5:<!ENTITY fullscreenButton.enterfullscreenlabel "Full Screen">
toolkit/locales/en-US/chrome/global/videocontrols.dtd:6:<!ENTITY fullscreenButton.exitfullscreenlabel "Exit Full Screen">
Comment 28 (Away 6/25-7/4) Jared Wein [:jaws] (reviews and needinfo disabled until back) 2012-04-10 13:38:16 PDT
Comment on attachment 613473 [details] [diff] [review]
Bug 705234 - Corrected inconsistent use of "full-screen", "full screen" and "Full-screen"

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

The string changes look good, but there are a bunch of accidental changes that were made to nsDocument.cpp. Also, I don't know why we wouldn't change the labels of the following entities:

> browser/locales/en-US/chrome/browser/browser.dtd:80:<!ENTITY fullScreenCmd.label "Full Screen">
> browser/locales/en-US/chrome/browser/browser.dtd:91:<!ENTITY fullScreenExit.label "Exit Full Screen Mode">
> browser/locales/en-US/chrome/browser/browser.dtd:466:<!ENTITY videoFullScreen.label       "Full Screen">
> mobile/android/locales/en-US/chrome/browser.properties:206:contextmenu.fullScreen=Full Screen
> mobile/xul/locales/en-US/chrome/browser.dtd:108:<!ENTITY contextFullScreen.label      "Full Screen">
> toolkit/locales/en-US/chrome/global/videocontrols.dtd:5:<!ENTITY fullscreenButton.enterfullscreenlabel "Full Screen">
> toolkit/locales/en-US/chrome/global/videocontrols.dtd:6:<!ENTITY fullscreenButton.exitfullscreenlabel "Exit Full Screen">
Comment 29 (Away 6/25-7/4) Jared Wein [:jaws] (reviews and needinfo disabled until back) 2012-05-07 21:07:24 PDT
Abhishek, is there anything I can do here to help you?
Comment 30 Chris Pearce (:cpearce) 2012-05-07 21:24:21 PDT
I'm changing most (all?) of the user-facing instances of "full-screen" to "fullscreen" in bug 716107. This should land in time for the FF15 train. Once that lands, we can rebase and remove the remaining instances in the code and anything I missed.
Comment 31 Abhishek Bhatnagar 2012-05-15 16:36:06 PDT
Hi guys, I apologize but I had been without a proper dev computer for the last few weeks and hence had stayed away from a lot of my work.

Just to be clear, I see 716107 has been resolved; so should I patch my moz-central with that to see what differences remain and then fix them?

I assume submitting this patch again with the recommended changes won't be helpful.
Comment 32 toe_head2001 2015-03-25 16:21:48 PDT
Created attachment 8583449 [details]
Screenshots of inconsistant use of Fullscreen

It seems HTML5 features use "Fullscreen", but fullscreen for the browser itself still uses "Full Screen".
Comment 33 Matej Novak [:matej] 2015-03-26 05:57:54 PDT
I still see "Enter Full Screen" under the View menu on Mac. Is updating this still in progress?

Note You need to log in before you can comment on or make changes to this bug.