Sevaan Franks [:sevaan] Reporter
	Description • 10 years ago

The following about: pages do not have page titles. Suggested titles are included:

about:				» About Firefox* 
about:buildconfig 		» Build Config
about:config 			» Configuration
about:license 			» Licenses
about:memory 			» Memory
about:plugins 			» Plugins
about:privatebrowsing 		» Start Private Browsing
about:rights 			» About Your Rights
about:sync-log			» Sync Log
about:webrtc			» WebRTC

* or Aurora, or Nightly.

	:Gijs (he/him)
	Comment 1 • 10 years ago

(In reply to Sevaan Franks [:sevaan] from comment #0)
> Created attachment 8470861 [details]
> Example of About: Page Title
> 
> The following about: pages do not have page titles.

Some do:

> about:plugins 			» Plugins

This has the title "About Plugins"

> about:privatebrowsing 		» Start Private Browsing

This has the title "Would you like to start private browsing" in a normal window, and "New Tab" in private browsing windows (where I presume we don't want it to say "Start Private Browsing"?).

> about:webrtc			» WebRTC

This has the title "WebRTC Internals"


Also,

> about:sync-log			» Sync Log

This is a file listing. I don't know that we can really give it a title, or if calling it "Sync Log" when it clearly isn't the sync log, but a directory listing containing sync logs, is useful.

	Sevaan Franks [:sevaan] Reporter
	Comment 2 • 10 years ago

Sorry, yes. Those page titles need updating too. Should I file a separate bug for those?

We'd like to remove some of the redundant "abouts" in the titles, hence the change to plugins.

"Start Private Browing" fits better in a tab than the entire question "Would you like to start private browsing?"

"WebRTC Internals" I am okay with keeping...but just thought WebRTC was pithier.

If we can't change the title for Sync Log, that's okay...but "Sync Logs" (the plural) does work for the page.

	:Gijs (he/him)
	Comment 3 • 7 years ago

Please don't set the good first bug keyword unless there is also a mentor for the bug. Can you mentor this bug? If so, please add yourself in the mentor field and provide a comment indicating how the bug should be fixed.

	Sebastian Zartner [:sebo]
	Comment 4 • 7 years ago

Thank you for the info, Gijs! I never mentored a bug so far, though I know the places which need to be touched. So, I'll try it.

Anyone interested in this bug should start looking for the files behind the about: pages. One that already has a proper title is the troubleshooting information page:

https://dxr.mozilla.org/mozilla-central/rev/7dddbd85047c6dc73ddbe1e423cd643a217845b3/toolkit/content/aboutSupport.xhtml#17

The title for it is stored in a DTD file here:

https://dxr.mozilla.org/mozilla-central/rev/7dddbd85047c6dc73ddbe1e423cd643a217845b3/toolkit/locales/en-US/chrome/global/aboutSupport.dtd#5

Other pages are missing this information, e.g. about:memory, or about:performance, and instead show their URI as title:

https://dxr.mozilla.org/mozilla-central/rev/7dddbd85047c6dc73ddbe1e423cd643a217845b3/toolkit/components/aboutmemory/content/aboutMemory.xhtml
https://dxr.mozilla.org/mozilla-central/rev/7dddbd85047c6dc73ddbe1e423cd643a217845b3/toolkit/components/aboutperformance/content/aboutPerformance.xhtml

So, to add a title to those pages, you first have to add an entity to the related DTD file (or create a new file if it doesn't exist yet) and then reference it within the <title> element of the about:* page.

An updated list of pages not having a proper title plus title suggestions are:

about:			» About Firefox* 
about:buildconfig	» Build Configuration
about:checkerboard	» Checkerboard Analyzer
about:config		» Configuration
about:memory		» Memory Analyzer
about:performance	» Performance Analyzer
about:studies		» Studies
about:sync-log		» Profile Synchronization Log
about:url-classifier	» URL Classifier

* or Beta, or Firefox Developer Edition, or Nightly. To get the right name use the &brandShortName; entity in the title string. See https://dxr.mozilla.org/mozilla-central/rev/7dddbd85047c6dc73ddbe1e423cd643a217845b3/browser/locales/en-US/chrome/browser/aboutHealthReport.dtd#6 as an example.

There are some pages, which could profit from renaming their title. For example, some of them have a redundant "About" like about:plugins.

Sebastian

	Neville
	Comment 5 • 7 years ago

Hi there, thought I might try this one out as my first Mozilla bug. Based on comments above I have made some changes:

about:			» About Firefox* 
Adjusted dtd and html, using &brandShortName;

about:buildconfig	» Build Configuration
Uses a different template system. Manually set as it didn't seem to get along with xml/dtd at all.

about:checkerboard	» Checkerboard Analyzer
Adjusted (added dtd, registered in jar manifest).

about:config		» Configuration
Adjusted dtd and html.

about:memory		» Memory Analyzer
Adjusted (added dtd). Page title is adjusted in aboutMemory.js when certain options are chosen. Updated aboutMemory.js to take original title from dtd.

about:performance	» Performance Analyzer
Adjusted (added dtd, registered in jar manifest).

about:studies		» Studies
Is packed in the extensions section of browser (mozilla-central/browser/extensions/shield-recipe-client/content/about-studies). Have left as-is, seems like it should be handled independently in a shield-recipe-client based bug perhaps?

about:sync-log		» Profile Synchronization Log
I think the HTML for this is generated from: https://dxr.mozilla.org/mozilla-central/source/netwerk/streamconv/converters/nsIndexedToHTML.cpp so I guess originally from C++ code. I haven't adjusted this.
 
about:url-classifier	» URL Classifier
Adjusted, added title property to dtd.

If this seems ok, I'll make a patch with these changes, otherwise I'm all ears for feedback. Thanks!

	Sebastian Zartner [:sebo]
	Comment 6 • 7 years ago

(In reply to Neville from comment #5)
> about:			» About Firefox* 
> Adjusted dtd and html, using &brandShortName;

Good.

> about:buildconfig	» Build Configuration
> Uses a different template system. Manually set as it didn't seem to get
> along with xml/dtd at all.

I didn't try that myself yet, but if it's not possible to use a DTD in that case, I guess it's better than nothing to manually set the title.
Just for reference, the file is available at https://dxr.mozilla.org/mozilla-central/rev/a9d372645a32b8d23d44244f351639af9d73b96a/toolkit/content/buildconfig.html.

> about:checkerboard	» Checkerboard Analyzer
> Adjusted (added dtd, registered in jar manifest).

Good.

> about:config		» Configuration
> Adjusted dtd and html.

Good.

> about:memory		» Memory Analyzer
> Adjusted (added dtd). Page title is adjusted in aboutMemory.js when certain
> options are chosen. Updated aboutMemory.js to take original title from dtd.

Great! You're right, e.g. the title is changed when you load a previously saved memory dump.

> about:performance	» Performance Analyzer
> Adjusted (added dtd, registered in jar manifest).

Good.

> about:studies		» Studies
> Is packed in the extensions section of browser
> (mozilla-central/browser/extensions/shield-recipe-client/content/about-
> studies). Have left as-is, seems like it should be handled independently in
> a shield-recipe-client based bug perhaps?

I'd say that this should also be covered in this bug, but let's ask the person responsible for that code. Mike, do you think this should be handled here or in a separate bug?

> about:sync-log		» Profile Synchronization Log
> I think the HTML for this is generated from:
> https://dxr.mozilla.org/mozilla-central/source/netwerk/streamconv/converters/
> nsIndexedToHTML.cpp so I guess originally from C++ code. I haven't adjusted
> this.

Right. I assumed that's hard to do, as it uses the same code as when you display any folder of the file system. It may be possible to adjust it to accept customized titles, though that's probably not worth the effort. So let's keep it as is.

> about:url-classifier	» URL Classifier
> Adjusted, added title property to dtd.

Good.

> If this seems ok, I'll make a patch with these changes, otherwise I'm all
> ears for feedback. Thanks!

Yes, please do so!

Sebastian

	Michael Cooper [:mythmon]
	Comment 7 • 7 years ago

> > about:studies		» Studies
> > Is packed in the extensions section of browser
> > (mozilla-central/browser/extensions/shield-recipe-client/content/about-
> > studies). Have left as-is, seems like it should be handled independently in
> > a shield-recipe-client based bug perhaps?
> 
> I'd say that this should also be covered in this bug, but let's ask the
> person responsible for that code. Mike, do you think this should be handled
> here or in a separate bug?

Keeping this in one bug is fine.

We'll need to also patch the upstream source at https://github.com/mozilla/normandy/tree/master/recipe-client-addon. Neville, if you're comfortable doing so, please file a pull request against that Github repo. If not, I can port the patch from this bug to the other repo.

	Neville
	Comment 9 • 7 years ago

Thanks for the feedback folks! 

Mike, I'm happy to also patch the upstream at github, would be good to have some clarification on how you'd prefer it fixed. My (incomplete/incorrect?) understanding is the advantage to using .dtd files as part of the locale section is that if/when it's localized elsewhere, native language equivalents of these files will be used for naming instead. The recipe client addon doesn't have localization stuff yet, would you like me to add it or is it ok for it to depend on the toolkit's locale stuff?

I attempted to use hg bzexport to export a patch but it said it failed as I'm not assigned to this bug. I didn't realise it would try to submit the patch directly! I seems to have partly worked, hence the attachment. I was just trying it out sorry. I also notice that it includes all the additional files that have nothing to do with this patch but were included after pulling down the latest changes - should I not have done a pull? 

Cheers!

	Sebastian Zartner [:sebo]
	Comment 10 • 7 years ago

Your patch obviously includes unrelated changes. Can you please rebase it, so it only includes the changes relevant to this bug?

Sebastian

	Michael Cooper [:mythmon]
	Comment 11 • 7 years ago

(In reply to Neville from comment #9)
> Mike, I'm happy to also patch the upstream at github, would be good to have
> some clarification on how you'd prefer it fixed. My (incomplete/incorrect?)
> understanding is the advantage to using .dtd files as part of the locale
> section is that if/when it's localized elsewhere, native language
> equivalents of these files will be used for naming instead. The recipe
> client addon doesn't have localization stuff yet, would you like me to add
> it or is it ok for it to depend on the toolkit's locale stuff?

For now the recipe client addon, and by extension about:studies, is not localized. We prioritized getting basic functionality done for English audiences before localization, since the early studies were targeted at only English speaking audiences. In the future we plan to revisit this to add support for localization.

It would be fine to make this change English-only for the recipe client addon. I would also be quite happy if you wanted to introduce localization to about:studies, but I assume that is too much work for this bug. I don' thave any opinions about particular methods you might use.

	Neville
	Comment 13 • 7 years ago

Yep sorry about that. I thought I'd get a chance to review the patch before upload, and also thought the patch might just be what was in hg outgoing. This one contains just the files I've altered.

For about:studies I have simply adjusted the title in the html for now. I'll look further into localization methods before making any more serious changes here.

Contrary to my earlier comment, I have now updated about:buildconfig to use a .dtd from locales also.

	Sebastian Zartner [:sebo]
	Comment 14 • 7 years ago

Comment on attachment 8901035 [details] [diff] [review]
Add page title to about: pages that do not have a title

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

Looks very good already. Only the buildconfig.xhtml needs to be fixed and the reference to the old .html be replaced by the new .xhtml one at https://dxr.mozilla.org/mozilla-central/rev/892c8916ba32b7733e06bfbfdd4083ffae3ca028/docshell/base/nsAboutRedirector.cpp#43.

Setting this to r-, because without the changes above this can't be shipped.

Sebastian

::: toolkit/content/buildconfig.xhtml
@@ +13,5 @@
> +#filter substitution
> +#include @TOPOBJDIR@/source-repo.h
> +<html xmlns="http://www.w3.org/1999/xhtml">
> +<head>
> +  <meta charset="UTF-8">

To be a valid XML, this tag and the other <meta> and <link> tags need to be closed.

	Neville
	Comment 16 • 7 years ago

Thanks Sebastian, 

I've updated the patch with your changes. Once I renamed buildconfig I should have searched the whole project for references, is this how you picked it up or are there particular tests that I should have been running?

	Sebastian Zartner [:sebo]
	Comment 17 • 7 years ago

(In reply to Neville from comment #16)
> Thanks Sebastian, 
> 
> I've updated the patch with your changes. Once I renamed buildconfig I
> should have searched the whole project for references, is this how you
> picked it up or are there particular tests that I should have been running?

First, I've simply accessed all the pages in the browser to test them. There I saw that about:buildconfig failed to load.
Then I searched for what was causing the errors and found that piece of code. Once I changed it and compiled again I saw that it threw XML errors and fixed them.

I still need to do the review, which may take a bit as I'm quite busy the next days. I set myself as reviewer again, so I don't forget about it.

Sebastian

	Sebastian Zartner [:sebo]
	Comment 18 • 7 years ago

Comment on attachment 8901620 [details] [diff] [review]
Add page title to about: pages that do not have a title

>diff --git a/toolkit/locales/jar.mn b/toolkit/locales/jar.mn
>--- a/toolkit/locales/jar.mn
>+++ b/toolkit/locales/jar.mn
>@@ -8,6 +8,10 @@
> % locale global @AB_CD@ %locale/@AB_CD@/global/
>   locale/@AB_CD@/global/about.dtd                       (%chrome/global/about.dtd)
>   locale/@AB_CD@/global/aboutAbout.dtd                  (%chrome/global/aboutAbout.dtd)
>+  locale/@AB_CD@/global/aboutBuildConfig.dtd                  (%chrome/global/aboutBuildConfig.dtd)
>+  locale/@AB_CD@/global/aboutCheckerboard.dtd                       (%chrome/global/aboutCheckerboard.dtd)
>+  locale/@AB_CD@/global/aboutMemory.dtd                       (%chrome/global/aboutMemory.dtd)
>+  locale/@AB_CD@/global/aboutPerformance.dtd                       (%chrome/global/aboutPerformance.dtd)
>   locale/@AB_CD@/global/aboutReader.properties          (%chrome/global/aboutReader.properties)
>   locale/@AB_CD@/global/aboutRights.dtd                 (%chrome/global/aboutRights.dtd)
>   locale/@AB_CD@/global/aboutNetworking.dtd             (%chrome/global/aboutNetworking.dtd)

Nit: You should align the indentation with the one of the other entries here. With that it's r+.

	Neville
	Comment 20 • 7 years ago

Ah, the perils of a narrow editor window!

I have amended the patch. Sorry for the delay.

Thanks!

	:Gijs (he/him)
	Comment 21 • 7 years ago

Comment on attachment 8904415 [details] [diff] [review]
Add page title to about: pages that do not have a title

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

Can you use "hg mv --after" (see "hg help mv" for details) to ensure that mercurial knows you moved buildconfig.html to buildconfig.xhtml, and we preserve per-line history where the lines don't change? :-)

Also, when you next put up a patch, can you add a review flag (select "?" in the dropdown and put sebo or my email in the text field)

	Sebastian Zartner [:sebo]
	Comment 22 • 7 years ago

Also, I've just realized from reading through https://hg.mozilla.org/mozilla-central/file/d225dbb6b59f/browser/components/about/AboutRedirector.cpp (which are not listed in about:about) that there are two more about: pages missing a title:

about:blocked
about:tabcrashed

The first should have "Deceptive Site" as title and the latter "Tab Crashed".
Is it possible you add a proper title for them, too?

Sebastian

	:Gijs (he/him)
	Comment 23 • 7 years ago

(In reply to Sebastian Zartner [:sebo] from comment #22)
> Also, I've just realized from reading through
> https://hg.mozilla.org/mozilla-central/file/d225dbb6b59f/browser/components/
> about/AboutRedirector.cpp (which are not listed in about:about) that there
> are two more about: pages missing a title:
> 
> about:blocked

This does get a title, see:

https://dxr.mozilla.org/mozilla-central/source/browser/base/content/blockedSite.xhtml#141-142

We don't care about the case where people manually load this, ditto for about:neterror and about:certerror.

> about:tabcrashed

Same: https://dxr.mozilla.org/mozilla-central/source/browser/base/content/aboutTabCrashed.js#61


So please don't touch these. Adding a hardcoded title will just cause additional flashing in the tabstrip / window title because we'll update the title twice.

	Sebastian Zartner [:sebo]
	Comment 24 • 7 years ago

(In reply to :Gijs from comment #23)
> (In reply to Sebastian Zartner [:sebo] from comment #22)
> > Also, I've just realized from reading through
> > https://hg.mozilla.org/mozilla-central/file/d225dbb6b59f/browser/components/
> > about/AboutRedirector.cpp (which are not listed in about:about) that there
> > are two more about: pages missing a title:
> > 
> > about:blocked
> 
> This does get a title
> 
> ...
> 
> > about:tabcrashed
> 
> Same

Ah, right, missed that. Sorry!

Sebastian

	Sebastian Zartner [:sebo]
	Comment 25 • 7 years ago

Neville, are you still interested in finishing up your patch? It's looks like you were very close. The last missing bit was mentioned by Gijs in comment 21.

Sebastian

	Matthew N. [:MattN]
	Comment 26 • 7 years ago

Comment on attachment 8900587 [details] [diff] [review]
Clean/shorten about: page titles

The last patch also needs rebasing now.

	Preeti[:preeti]
	Comment 27 • 6 years ago

Hello everyone! I would like to work on this bug in case no one is working on this. Can someone please guide me on how to proceed? Thanks in advance!

	Yushan Lin
	Comment 28 • 6 years ago

Hi, I'm interested in working on this bug.

	Pulsebot
	Comment 34 • 6 years ago

Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/0649bb18323b
Add a <title> to about:checkerboard and about:memory, r=Gijs