Closed Bug 1199760 Opened 4 years ago Closed 4 years ago

"Private Windows" shouldn't be capitalized in about:privatebrowsing

Categories

(Firefox :: Private Browsing, defect, P1, trivial)

defect

Tracking

()

RESOLVED FIXED
Firefox 43
Iteration:
43.2 - Sep 7
Tracking Status
firefox42 --- fixed
firefox43 --- fixed

People

(Reporter: dao, Assigned: dao)

References

Details

(Whiteboard: [fxprivacy])

Attachments

(1 file)

Attached patch patchSplinter Review
"Private Window(s)" is not a proper name and shouldn't be capitalized unless it labels an element such as a button or a menu item where title capitalization should be used.
Attachment #8654294 - Flags: review?(paolo.mozmail)
Comment on attachment 8654294 [details] [diff] [review]
patch

This may have to go through a formal copy review - we'll triage this bug next week at our daily meeting. Clearing the review flag in the meantime.
Attachment #8654294 - Flags: review?(paolo.mozmail)
Whiteboard: [fxprivacy][triage]
(In reply to :Paolo Amadini from comment #1)
> Comment on attachment 8654294 [details] [diff] [review]
> patch
> 
> This may have to go through a formal copy review - we'll triage this bug
> next week at our daily meeting. Clearing the review flag in the meantime.

I have no idea what this means. Why can you not review this like any other patch according to Mozilla's code review rules?
Flags: needinfo?(paolo.mozmail)
Blocks: 1188565
Flags: qe-verify-
Priority: -- → P3
Whiteboard: [fxprivacy][triage] → [fxprivacy]
Comment on attachment 8654294 [details] [diff] [review]
patch

I could definitely set the code review flag here, but this would not be enough for landing the patch, since it's a user interface change. I asked at today's meeting and we can flag Matej directly for UI review on this one.

I didn't set the r+ flag in order to avoid confusion, since we routinely land patches without explicit ui-review when they come directly from a specification we received from UX (the original text here came from a detailed copy review).

Changes, however, still require the UI review flag. Once you have it, feel free to land with rs=me, no need to ask for code review again since there's really no code change here.
Flags: needinfo?(paolo.mozmail)
Attachment #8654294 - Flags: ui-review?(matej)
I don't actually do UI review. I think that should be someone from the UX team, no?
(In reply to Matej Novak [:matej] from comment #4)
> I don't actually do UI review. I think that should be someone from the UX
> team, no?

I talked to Aislinn in our meeting and since she defers the decision to you anyways, we'd rather flag you directly than flag her, have her send a e-mail to you, and then get back to the bug and grant or deny the UI review accordingly. We don't have a dedicated "copy review" flag :-)
(In reply to :Paolo Amadini from comment #5)
> (In reply to Matej Novak [:matej] from comment #4)
> > I don't actually do UI review. I think that should be someone from the UX
> > team, no?
> 
> I talked to Aislinn in our meeting and since she defers the decision to you
> anyways, we'd rather flag you directly than flag her, have her send a e-mail
> to you, and then get back to the bug and grant or deny the UI review
> accordingly. We don't have a dedicated "copy review" flag :-)

Understood, and I'm happy to review and approve copy in a bug comment (like I normally do), but since I don't deal with code, I don't trust myself to give a UI review, in case there's something else going on in there that I miss.

To be honest, I'm not even sure what I'm supposed to look at or do with the attachment in comment 3.
(In reply to :Paolo Amadini from comment #3)
> Changes, however, still require the UI review flag.

I'm just applying established rules and common sense here, and it's easier to have engineers rubber-stamp such trivial changes since they have to review the code anyway. If you think this case is more complicated, it would be helpful if you could voice your concerns. I.e. I'm not sure where you think comment 0 might be wrong. Is it that Private Windows isn't a proper name or that in English, words that aren't proper names shouldn't be capitalized randomly?
(In reply to Matej Novak [:matej] from comment #6)
> Understood, and I'm happy to review and approve copy in a bug comment (like
> I normally do), but since I don't deal with code, I don't trust myself to
> give a UI review, in case there's something else going on in there that I
> miss.

Don't worry, there's nothing else going on :-) But I can definitely handle the bug flags for you prefer to simply answer the question in a comment.

I think the question for you is the following: is the capitalization of "Private Windows" in the about:privatebrowsing landing page a mistake, something we overlooked in the copy review, or was it intentional to call attention to it?

The full copy we currently have is "Private Windows now block parts of the page that may track your browsing activity."

The updated copy is "Private windows now block parts of the page that may track your browsing activity."

I think Dão's proposal is reasonable but we'd like to check with you as this is a very high visibility page. In other cases we would feel comfortable doing a UI review internally without this degree of scrutiny.
(In reply to :Paolo Amadini from comment #8)
> (In reply to Matej Novak [:matej] from comment #6)
> > Understood, and I'm happy to review and approve copy in a bug comment (like
> > I normally do), but since I don't deal with code, I don't trust myself to
> > give a UI review, in case there's something else going on in there that I
> > miss.
> 
> Don't worry, there's nothing else going on :-) But I can definitely handle
> the bug flags for you prefer to simply answer the question in a comment.
> 
> I think the question for you is the following: is the capitalization of
> "Private Windows" in the about:privatebrowsing landing page a mistake,
> something we overlooked in the copy review, or was it intentional to call
> attention to it?
> 
> The full copy we currently have is "Private Windows now block parts of the
> page that may track your browsing activity."
> 
> The updated copy is "Private windows now block parts of the page that may
> track your browsing activity."
> 
> I think Dão's proposal is reasonable but we'd like to check with you as this
> is a very high visibility page. In other cases we would feel comfortable
> doing a UI review internally without this degree of scrutiny.

Got it. Thank you.

In general, I tend to capitalize anything that's a feature or that appears in a menu (even when outside the context of the browser). In this case the menu item says "New Private Window," so I would be inclined to leave it capitalized.

On the other hand, I didn't capitalize "private window" on this page (for some reason): https://www.mozilla.org/en-US/firefox/desktop/trust/

So for consistency, let's lowercase it per the recommendation in comment 0.
Comment on attachment 8654294 [details] [diff] [review]
patch

(In reply to Matej Novak [:matej] from comment #9)
> In general, I tend to capitalize anything that's a feature or that appears
> in a menu (even when outside the context of the browser). In this case the
> menu item says "New Private Window," so I would be inclined to leave it
> capitalized.
> 
> On the other hand, I didn't capitalize "private window" on this page (for
> some reason): https://www.mozilla.org/en-US/firefox/desktop/trust/
> 
> So for consistency, let's lowercase it per the recommendation in comment 0.

I knew there was some good reasoning, thanks for sharing it.

Let's land this and uplift to Firefox 42.
Attachment #8654294 - Flags: ui-review?(matej)
Attachment #8654294 - Flags: ui-review+
Attachment #8654294 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/cd0e4d4aff3d
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
Iteration: --- → 43.2 - Sep 7
Comment on attachment 8654294 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: New about:privatebrowsing landing page
[User impact if declined]: Small difference in capitalization
[Describe test coverage new/current, TreeHerder]: Landed on m-c
[Risks and why]: None
[String/UUID change made/needed]: Changed entity contents but not entity name

Francesco, I think this small change requires the l10n review as well.
Flags: needinfo?(francesco.lodolo)
Attachment #8654294 - Flags: approval-mozilla-aurora?
Looks like we just need a joint uplift and l10n approval from Sylvestre on this one.
Flags: needinfo?(francesco.lodolo)
l10n changes are the call of Flod (or Pike).
If they are ok, I am ok with the uplift. I am pretty sure they won't be ok :p
Flags: needinfo?(francesco.lodolo)
Francesco mentioned he's on PTO and I've tried to needinfo Axel on another bug but didn't get a response yet.

This is the least important of all our fixes to about:privatebrowsing, though we'd like to uplift this before we get in touch with localizers with a summary of the changes. Sylvestre, can you help getting l10n review on this?
Flags: needinfo?(francesco.lodolo)
Flags: needinfo?(l10n)
I think this is OK. Paolo, do you want to follow up in https://groups.google.com/forum/#!topic/mozilla.dev.l10n/5wPH8xOLIVE?

I'll also clear my other needinfo, feel free to lump in additions on both to that post?
Flags: needinfo?(l10n)
Comment on attachment 8654294 [details] [diff] [review]
patch

If Axel thinks it is ok, let's take it then :)
Attachment #8654294 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Keywords: checkin-needed
Priority: P3 → P1
You need to log in before you can comment on or make changes to this bug.