1203547 - Review and remove unused strings

Status: RESOLVED WONTFIX

(Firefox :: General, defect)

Description

[Axel Hecht [:Pike]]

We talked about this, and it turned out that I was able to script something hackerish, and at a brief glance, I found a few strings that aren't use anymore.

Not yet a concise list of what we're using, though.

browser/locales/en-US/chrome/browser/aboutRobots.dtd
 robots.imgtitle
browser/locales/en-US/chrome/browser/aboutSessionRestore.dtd
 welcomeback2.tabtitle
browser/locales/en-US/chrome/browser/browser.dtd
 deleteCmd.key devToolbarOtherToolsButton.label devtoolsConnect.accesskey getUserMedia.audioCapture.label remoteWebConsoleCmd.label sharePageCmd.commandkey social.activated.description social.activated.undo.accesskey social.activated.undo.label social.chatBar.accesskey social.chatBar.label social.closeNotificationItem.label social.ok.accesskey social.ok.label socialToolbar.title syncBrand.shortName.label trackingContentBlocked.block.accesskey trackingContentBlocked.block.label trackingContentBlocked.disabled.message trackingContentBlocked.learnMore trackingContentBlocked.message trackingContentBlocked.moreinfo trackingContentBlocked.options trackingContentBlocked.unblock2.accesskey trackingContentBlocked.unblock2.label viewTabGroups.accesskey
browser/locales/en-US/chrome/browser/devtools/app-manager.dtd
 projects.addApp projects.appDetails projects.localApps
browser/locales/en-US/chrome/browser/devtools/connection-screen.dtd
 connectionError
browser/locales/en-US/chrome/browser/devtools/debugger.dtd
 debuggerUI.clearButton debuggerUI.clearButton.tooltip debuggerUI.closeButton.tooltip debuggerUI.startTracing debuggerUI.tabs.traces
browser/locales/en-US/chrome/browser/devtools/inspector.dtd
 inspector.selectButton.tooltip inspectorSearchHTML.label3
browser/locales/en-US/chrome/browser/devtools/netmonitor.dtd
 netmonitorUI.summary.size
browser/locales/en-US/chrome/browser/devtools/performance.dtd
 performanceUI.table.selfAlloc performanceUI.table.selfAlloc.tooltip performanceUI.table.totalAlloc
browser/locales/en-US/chrome/browser/devtools/scratchpad.dtd
 helpMenuWin.label resetContext2.accesskey resetContext2.label
browser/locales/en-US/chrome/browser/devtools/styleinspector.dtd
 selectedElementLabel
browser/locales/en-US/chrome/browser/devtools/toolbox.dtd
 options.usedeveditiontheme.label options.usedeveditiontheme.tooltip
browser/locales/en-US/chrome/browser/devtools/webaudioeditor.dtd
 webAudioEditorUI.inspectorTitle
browser/locales/en-US/chrome/browser/engineManager.dtd
 addEngine.accesskey
browser/locales/en-US/chrome/browser/newTab.dtd
 newtab.customize.suggested newtab.customize.topsites
browser/locales/en-US/chrome/browser/pageInfo.dtd
 generalSource permAllowSession
browser/locales/en-US/chrome/browser/places/editBookmarkOverlay.dtd
 editBookmarkOverlay.feedLocation.accesskey editBookmarkOverlay.feedLocation.label editBookmarkOverlay.siteLocation.accesskey editBookmarkOverlay.siteLocation.label
browser/locales/en-US/chrome/browser/places/places.dtd
 cmd.sortby_name.accesskey
browser/locales/en-US/chrome/browser/preferences/aboutPermissions.dtd
 plugins.label
browser/locales/en-US/chrome/browser/preferences/advanced.dtd
 offlineAppRemove.confirm offlineAppsList.height
browser/locales/en-US/chrome/browser/preferences/privacy.dtd
 suggestionSettings.accesskey
browser/locales/en-US/chrome/browser/setDesktopBackground.dtd
 closeWindow.key
browser/locales/en-US/chrome/browser/syncSetup.dtd
 changeOptions.label continueUsing.label setup.choicePage.existing2.label setup.choicePage.new.label setup.choicePage.title.label setup.newRecoveryKeyPage.description.label setup.newRecoveryKeyPage.title.label setup.successPage.title setup.tosAgree2.accesskey
mobile/android/locales/en-US/chrome/phishing.dtd
 safeb.palm.notforgery.label2
toolkit/locales/en-US/chrome/global/aboutSupport.dtd
 aboutSupport.installationHistoryTitle aboutSupport.updateHistoryTitle
toolkit/locales/en-US/chrome/global/commonDialog.dtd
 checkbox.label header.label message.label
toolkit/locales/en-US/chrome/global/console.dtd
 copyCmd.commandkey errColumn.label
toolkit/locales/en-US/chrome/global/customizeToolbar.dtd
 undoChanges.label
toolkit/locales/en-US/chrome/global/textcontext.dtd
 fillLoginMenu.accesskey fillLoginMenu.label fillUsernameMenu.accesskey fillUsernameMenu.label
toolkit/locales/en-US/chrome/global/viewSource.dtd
 findAgainCmd.accesskey findAgainCmd.label findOnCmd.accesskey findOnCmd.label
toolkit/locales/en-US/chrome/mozapps/extensions/extensions.dtd
 addon.homepage detail.creator.label detail.homepage.label detail.version.label progress.pause.tooltip
toolkit/locales/en-US/chrome/mozapps/extensions/xpinstallConfirm.dtd
 from.label
toolkit/locales/en-US/chrome/mozapps/help/help.dtd
 homeButton.label
toolkit/locales/en-US/chrome/mozapps/preferences/changemp.dtd
 setPassword.meter.loading setPassword.tokenName.label
toolkit/locales/en-US/chrome/mozapps/profile/profileSelection.dtd
 availprofiles.label profilename.label
toolkit/locales/en-US/chrome/mozapps/update/history.dtd
 date.header name.header state.header type.header
toolkit/locales/en-US/chrome/mozapps/update/updates.dtd
 downloading.intro license.instructions license.introText viewDetails.tooltip
toolkit/locales/en-US/chrome/passwordmgr/passwordManager.dtd
 addLogin.accesskey addLogin.label
webapprt/locales/en-US/webapprt/webapp.dtd
 deleteCmd.key redoCmd.key


Just had a laugh. https://github.com/mozilla/gecko-dev/commit/82d50f4e1ffb006877014c9899e6703a7abe8d34

Comment 1

[Axel Hecht [:Pike]]

Actually, not affecting l10n, but

layout/tools/layout-debug/ui/locale/en-US/layoutdebug.dtd
 app.author app.name.long app.name.short app.version

wasn't a false positive, so adding it.

Comment 2

[Axel Hecht [:Pike]]

Attachment: script used

This is the script I used, fwiw. It requires a current version of compare-locales and hglib to be installed.

Comment 3

[Axel Hecht [:Pike]]

Attachment: MozReview Request: bug 1203547, remove un-used strings in DTDs, r?dolske

bug 1203547, remove un-used strings in DTDs, r?dolske

First patch in a series of patches, up to preferences, in alphabetical order.

Comment 4

[Axel Hecht [:Pike]]

I did a pretty full test run on linux, https://treeherder.mozilla.org/#/jobs?repo=try&revision=b513e5d744b4, all green.

Dolske, I picked you for review because most of these strings date back to cvs history, some added without ever being used, some stopped being used back then. Also, some just stopped being used just recently.

Thus, this involves some cross-project good understanding of things. Or just faith in my investigations.

I haven't done the full patch yet, as I want to first see how the approach flies.

Investigations:

I commonly went by hg/git blame for the DTD, and then figured out which patch landed the string. And then checked if it got used as part of landing or not. If it was used, I used VCS to track when it stopped being used, and cross-checked the patch that the removal was missing the string removal.

Comment 5

[Justin Dolske [:Dolske]]

Comment on attachment 8694210 [details]
MozReview Request: bug 1203547, remove un-used strings in DTDs, r?dolske

https://reviewboard.mozilla.org/r/26709/#review24235

I took a skim through the removals, and looks at a few mildly surprising ones, but overall looks fine.

Not sure if this is super useful to do (ie, removals seem like a tiny fraction of our strings), but whatevs.

Might be worth thinking about landing this at the beginning of a Nightly cycle. Kinda feel like it's inevitable we'll miss some usage because of dumb code building string names in pieces, and that way we'd have maximal time to get reports of bustage.

Comment 6

[Marco Bonardo [:mak]]

any possibility we could convert that script to a localization-linter that runs on treeherder and notifies about unused strings?
Preventing them seems better than cleaning them up yearly.

Comment 7

[Axel Hecht [:Pike]]

This is part of the plan to keep currently not-used strings on purpose as long as we're shipping them somewhere.

I guess having a failing build whenever we use the crutch of pre-landed strings would probably annoy people.

Integrating this into a review linter that warns about new strings that don't get used would be a good win, though.

Comment 8

[Marco Bonardo [:mak]]

It could be part of the eslint (or a more general linting build including both) build.
Doesn't look like this happens often enough to annoy anyone, and I'm sure people would be happy to fix the code properly rather than annoyed by having to do it, as for any other mistake the harness can detect.

Comment 9

[:Gijs (he/him)]

Axel, any reason in particular this never landed?

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

The leave-open keyword is there and there is no activity for 6 months.
:Dolske, maybe it's time to close this bug?

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

The leave-open keyword is there and there is no activity for 6 months.
:Dolske, maybe it's time to close this bug?

Comment 12

[Mark Banner (:standard8)]

Closing as this hasn't gone anywhere, and there is now a newer effort in bug 1580981 that is happening.