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

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.)

*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...

(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.

(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.

(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.

"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.

(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.

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!)

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.

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.

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.

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.)

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".

(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)?

(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...

(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.

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?

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'.

(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

Corrects inconsistent use of "full-screen", "full screen", "Full-screen" and "Full-Screen". It leaves "Full Screen" as is as per bug comments.

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.

(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 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

(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.

Yes, these are English-specific spelling fixes; don't change the entity names.

Does not update Entity names as per discussion

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 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">

Abhishek, is there anything I can do here to help you?

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.

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.

It seems HTML5 features use "Fullscreen", but fullscreen for the browser itself still uses "Full Screen".

I still see "Enter Full Screen" under the View menu on Mac. Is updating this still in progress?

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.