Page MenuHomePhabricator
Differential D104869

Bug 1690340 - Part 2: Use the new naming for the developer tools menu items. r=jdescottes,mconley
ClosedPublic
Actions

Authored by gl on Feb 11 2021, 8:42 PM.
Tags
Referenced Files
Unknown Object (File)
Mon, May 5, 11:41 PM
Unknown Object (File)
Fri, May 2, 3:15 AM
Unknown Object (File)
Apr 2 2025, 11:03 PM
Unknown Object (File)
Mar 19 2025, 5:59 PM
Unknown Object (File)
Jan 19 2025, 2:29 PM
Unknown Object (File)
Jan 15 2025, 9:34 AM
Unknown Object (File)
Jan 13 2025, 2:58 PM
Unknown Object (File)
Oct 5 2024, 8:29 AM
Subscribers
flod
reviewbot

Details

Reviewers
jdescottes
mconley
Commits
rMOZILLACENTRALc86dc4f007dc: Bug 1690340 - Part 2: Use the new naming for the developer tools menu items.
Bugzilla Bug ID
1690340
Summary
  • Renames "Toggle Tools" to "Web Developer Tools"
  • Renames "Get More Tools" to "Extensions for Developers"

Diff Detail

Repository
rMOZILLACENTRAL mozilla-central
Lint
Lint Not Applicable
Unit
Tests Not Applicable
Build Status
Buildable 289543
Build 382491: arc lint + arc unit

Event Timeline

gl created this revision.Feb 11 2021, 8:42 PM
Herald added a project: secure-revision. · View Herald TranscriptFeb 11 2021, 8:42 PM
Harbormaster completed remote builds in B287176: Diff 398715.Feb 11 2021, 8:42 PM
phab-bot published this revision for review.Feb 11 2021, 8:42 PM
phab-bot changed the visibility from "Custom Policy" to "Public (No Login Required)".
phab-bot changed the edit policy from "Custom Policy" to "Restricted Project (Project)".
phab-bot removed a project: secure-revision.
Herald added a subscriber: flod. · View Herald TranscriptFeb 11 2021, 8:42 PM
gl added a child revision: D104870: Bug 1690340 - Part 3: Remove the "Work Offline" menu item from the developer tools menu. r=mconley,jdescottes.Feb 11 2021, 8:43 PM
reviewbot added a subscriber: reviewbot.Feb 11 2021, 9:01 PM

Code analysis found 1 defect in the diff 398715:

  • 1 defect found by code coverage analysis

In our previous code coverage analysis run, we found some files which had no coverage and are being modified in this patch:
Should they have tests, or are they dead code?

  • You can file a bug blocking Bug 1415824 for untested files that should be tested.
  • You can file a bug blocking Bug 1415819 for untested files that should be removed.

If you see a problem in this automated review, please report it here.

You can view these defects on the code-review frontend and on Treeherder.

mconley resigned from this revision.Feb 11 2021, 9:11 PM
mconley added inline comments.
devtools/client/locales/en-US/menus.properties
31–35

I can already hear @flod - instead of updating these, we should port them to Fluent first.

devtools/client/menus.js
65–66

Whatever imports this collection is, I think, going to have to know when it's populating the menubar menupopup, or the DevTools button menu / AppMenu DevTools menu (which are effectively the same thing). That way it can choose between the menubar.ftl string variants and the appmenu.ftl variants.

The reason for having different variants is that we're going to eventually sentence case the appmenu variants, while leaving the menubar variants in Title Case.

gl updated this revision to Diff 398809.Feb 11 2021, 11:05 PM
gl retitled this revision from Bug 1690340 - Part 3: Use the new naming and formating for the developer tools menu items. r=jdescottes,mconley to Bug 1690340 - Part 3: Use the new naming for the developer tools menu items. r=jdescottes,mconley.
gl edited the summary of this revision. (Show Details)
Harbormaster completed remote builds in B287243: Diff 398809.Feb 11 2021, 11:05 PM
reviewbot added a comment.Feb 11 2021, 11:33 PM

Code analysis found 1 defect in the diff 398809:

  • 1 defect found by code coverage analysis

In our previous code coverage analysis run, we found some files which had no coverage and are being modified in this patch:
Should they have tests, or are they dead code?

  • You can file a bug blocking Bug 1415824 for untested files that should be tested.
  • You can file a bug blocking Bug 1415819 for untested files that should be removed.

If you see a problem in this automated review, please report it here.

You can view these defects on the code-review frontend and on Treeherder.

gl updated this revision to Diff 398826.Feb 11 2021, 11:44 PM
Harbormaster completed remote builds in B287260: Diff 398826.Feb 11 2021, 11:44 PM
reviewbot added a comment.Feb 12 2021, 12:03 AM

Code analysis found 1 defect in the diff 398826:

  • 1 defect found by code coverage analysis

The analysis task source-test-node-devtools-tests failed, but we could not detect any issue.
Please check this task manually.

The analysis task source-test-mozlint-file-whitespace failed, but we could not detect any issue.
Please check this task manually.

The analysis task source-test-mozlint-eslint failed, but we could not detect any issue.
Please check this task manually.

In our previous code coverage analysis run, we found some files which had no coverage and are being modified in this patch:
Should they have tests, or are they dead code?

  • You can file a bug blocking Bug 1415824 for untested files that should be tested.
  • You can file a bug blocking Bug 1415819 for untested files that should be removed.

If you see a problem in this automated review, please report it here.

You can view these defects on the code-review frontend and on Treeherder.

flod added inline comments.Feb 12 2021, 2:53 AM
devtools/client/locales/en-US/menus.properties
31–35

I think it would be great, but Devtools is also kind a special case at this point :-( (I would still yell if it was DTDs).

More important, why "M" when it's not available in the label? This will results in Extensions for Developers (M).

gl updated this revision to Diff 398965.Feb 12 2021, 4:27 AM
Harbormaster completed remote builds in B287359: Diff 398965.Feb 12 2021, 4:27 AM
reviewbot added a comment.Feb 12 2021, 4:49 AM

Code analysis found 1 defect in the diff 398965:

  • 1 defect found by code coverage analysis

In our previous code coverage analysis run, we found some files which had no coverage and are being modified in this patch:
Should they have tests, or are they dead code?

  • You can file a bug blocking Bug 1415824 for untested files that should be tested.
  • You can file a bug blocking Bug 1415819 for untested files that should be removed.

If you see a problem in this automated review, please report it here.

You can view these defects on the code-review frontend and on Treeherder.

jdescottes accepted this revision.Feb 12 2021, 6:17 PM
jdescottes added a project: testing-exception-ui (Only changes ui styling, images, or localized strings).
jdescottes added inline comments.
devtools/client/locales/en-US/menus.properties
31–35

That's the same access key as browserToolboxMenu. Maybe F?

This revision is now accepted and ready to land.Feb 12 2021, 6:17 PM
gl updated this revision to Diff 400535.Feb 16 2021, 6:04 PM
gl retitled this revision from Bug 1690340 - Part 3: Use the new naming for the developer tools menu items. r=jdescottes,mconley to Bug 1690340 - Part 2: Use the new naming for the developer tools menu items. r=jdescottes,mconley.
Harbormaster completed remote builds in B288783: Diff 400535.Feb 16 2021, 6:04 PM
gl marked 3 inline comments as done.Feb 16 2021, 6:05 PM
flod added inline comments.Feb 16 2021, 6:07 PM
devtools/client/locales/en-US/menus.properties
31–35

F is still not available in the label. We only have one case AFAIK of this (Inspect element with Q), and I don't think we shouldcreate more.

reviewbot added a comment.Feb 16 2021, 6:22 PM

Code analysis found 1 defect in the diff 400535:

  • 1 defect found by code coverage analysis

In our previous code coverage analysis run, we found some files which had no coverage and are being modified in this patch:
Should they have tests, or are they dead code?

  • You can file a bug blocking Bug 1415824 for untested files that should be tested.
  • You can file a bug blocking Bug 1415819 for untested files that should be removed.

If you see a problem in this automated review, please report it here.

You can view these defects on the code-review frontend and on Treeherder.

jdescottes added inline comments.Feb 16 2021, 7:39 PM
devtools/client/locales/en-US/menus.properties
31–35

There is the "f" from "for"? Should it be only from the first word? Or is the case important?

flod added inline comments.Feb 16 2021, 7:56 PM
devtools/client/locales/en-US/menus.properties
31–35

Sorry, my brain got completely thrown off by F vs f :-(

Having said that, please match the case (so, f in this case). F would still work, but no point in relying on fallback.

gl marked 2 inline comments as done.Feb 16 2021, 10:29 PM
gl updated this revision to Diff 400669.Feb 16 2021, 10:34 PM
Harbormaster completed remote builds in B288871: Diff 400669.Feb 16 2021, 10:34 PM
Closed by commit rMOZILLACENTRALc86dc4f007dc: Bug 1690340 - Part 2: Use the new naming for the developer tools menu items. (authored by Gabriel Luong <gabriel.luong@gmail.com>). · Explain WhyFeb 16 2021, 10:43 PM
This revision was automatically updated to reflect the committed changes.
Gabriel Luong <gabriel.luong@gmail.com> added a commit: rMOZILLACENTRALc86dc4f007dc: Bug 1690340 - Part 2: Use the new naming for the developer tools menu items..
reviewbot added a comment.Feb 16 2021, 10:50 PM

Code analysis found 1 defect in the diff 400669:

  • 1 defect found by code coverage analysis

In our previous code coverage analysis run, we found some files which had no coverage and are being modified in this patch:
Should they have tests, or are they dead code?

  • You can file a bug blocking Bug 1415824 for untested files that should be tested.
  • You can file a bug blocking Bug 1415819 for untested files that should be removed.

If you see a problem in this automated review, please report it here.

You can view these defects on the code-review frontend and on Treeherder.

Harbormaster completed remote builds in B289543: Diff 400689.Mar 8 2021, 9:18 AM

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
devtools/
client/
locales/
en-US/
8 lines--
6 lines66%
Loading...

Diff 400689

View Options

devtools/client/locales/en-US/menus.properties

Loading...
View Options

devtools/client/menus.js

Loading...
Privacy · Cookies · Legal