[Desktop] Custom profile image `manageProfile` integration by chrislacy · Pull Request #39432 · brave/brave-core

chrislacy · 2026-08-27T02:03:07Z

Resolves [Desktop] Add shared Lit custom profile image row brave-browser#58430
Resolves [Desktop] Add manageProfile custom profile image selection UI brave-browser#58372

Screenshots

Screenshot_2026-08-27.at.12.01.32.mp4

String Usage

IDS_CUSTOM_PROFILE_IMAGE_TITLE
IDS_CUSTOM_PROFILE_IMAGE_UPLOAD_ACTION
IDS_CUSTOM_PROFILE_IMAGE_UPLOAD_TOOLTIP:

IDS_CUSTOM_PROFILE_IMAGE_REPLACE_ACTION
IDS_CUSTOM_PROFILE_IMAGE_REPLACE_TOOLTIP:

IDS_CUSTOM_PROFILE_IMAGE_REMOVE_ACTION
IDS_CUSTOM_PROFILE_IMAGE_REMOVE_TOOLTIP:

IDS_CUSTOM_PROFILE_IMAGE_INVALID_IMAGE:

IDS_CUSTOM_PROFILE_IMAGE_SELECTED_PREVIEW_LABEL:

- handle image validation, preview replacement, and object URL cleanup - keep strings and feature state owned by consumers - package the row only when custom profile image WebUI support is enabled

- insert the feature-gated row between the theme and avatar pickers - supply labels through generated BraveSettingsStrings - cover feature state, image selection, and preview lifecycle in Settings tests

- reuse the upload label as hover text for the icon button - verify the tooltip matches its accessible name in Settings tests

github-actions · 2026-08-27T02:06:48Z

📋 Code Owners Summary

13 file(s) changed, 3 with assigned owners

2 team(s) affected: @brave/chromium-src-web-reviewers, @brave/string-reviewers-team

Owners and Their Files

`@brave/string-reviewers-team` — 2 file(s)

`@brave/chromium-src-web-reviewers` — 1 file(s)

chromium_src/chrome/test/data/webui/settings/people_page_manage_profile_test.ts

- add custom profile image replace tooltip - remove unused custom profile image preview label

fallaciousreasoning

hey looking pretty good. I've left a few comments. One thing I was wondering is whether we need the separate remove/replace state. Could we just leave it as an upload custom image button and let the user pick one of the default profiles?

brave-builds · 2026-08-27T04:00:27Z

A Storybook has been deployed to preview UI for the latest push

Co-authored-by: Jay Harris <jay.harris@outlook.co.nz>

Compare the two section parents directly, then use the shared parent to insert the custom image section.

Replace Chromium styling usage with corresponding Leo items.

Compute the row state from `localPreviewUrl_` so upload/removal paths don't maintain duplicate state.

- keep the title in the DOM for `aria-labelledby` - let the reflected hide-title attribute control its layout and visibility

- resolve labels and tooltips in Lit template so consumers don't need localization-only properties - update Settings test to verify rendered localized content

- use reactive `localPreviewUrl_` as the single source of truth - remove the public state API and unused `state-*` class - centralize dynamic upload/replace strings

- share preview styles through `#preview` - size the selected indicator with `--leo-icon-size` - remove unused button-specific styling

- add the Lit template replacement markers - replace the static selected-preview label with `$i18n{...}`

This reverts commit 9f86316.

chrislacy · 2026-08-28T03:41:46Z

(Note: the most recent round of feedback came via commit comments. I won't reply to these commit comments as I'm unsure if they will survive a potential future branch rebase)

chrislacy · 2026-08-28T03:48:35Z

I tried the suggested $i18n{...} replacement in 9f86316.

It could be an issue with my implementation, but the placeholder was not expanded at runtime. The literal "$i18n{CUSTOM_PROFILE_IMAGE_SELECTED_PREVIEW_LABEL}" became the image’s accessible name:

642524181-39a4caeb-ab1f-41e4-b926-fbcd0199d6c4

I reverted this change with the subsequent 42727e5.

fallaciousreasoning

lgtm % nits and a followup

fallaciousreasoning · 2026-08-28T06:08:35Z

+async function settleRow(row: BrCustomProfileImageRowElement) {
+  await microtasksFinished()
+  await row.updateComplete
+  await microtasksFinished()
+  await row.updateComplete
+}


any chance we could give ourselves an event we can wait for instead? If not, then maybe a comment about why we need to wait twice?

fallaciousreasoning · 2026-08-28T06:09:43Z

+    createdUrls = []
+    revokedUrls = []
+    originalCreateObjectUrl = URL.createObjectURL
+    originalRevokeObjectUrl = URL.revokeObjectURL
+    originalDecode = HTMLImageElement.prototype.decode
+    URL.createObjectURL = () => {
+      const url = `blob:custom-profile-image-${createdUrls.length + 1}`
+      createdUrls.push(url)
+      return url
+    }
+    URL.revokeObjectURL = (url: string) => revokedUrls.push(url)
+    HTMLImageElement.prototype.decode = () => Promise.resolve()


do we need all these mocks?

fallaciousreasoning · 2026-08-28T06:11:28Z

+    assertDeepEquals(createdUrls, revokedUrls)
+  })
+
+  test('DisconnectWhileDecodePendingRevokesUrl', async function() {


I think we don't need to stress too much about testing we revoke the urls. Probably safe to remove a bunch of these.

fallaciousreasoning · 2026-08-28T06:15:25Z

+  protected onUploadClick_() {
+    this.shadowRoot.querySelector<HTMLInputElement>('#fileInput')!.click()
+  }


I think you can declare a $ property on the interface with an HTMLInputLEement on it and Lit will automatically wire it up.

Have a look at ui/webui/resources/cr_elements/cr_button/cr_button.ts for an example

fallaciousreasoning · 2026-08-28T06:16:27Z

+  }
+
+  private clearLocalPreview_() {
+    ++this.uploadAttemptId_


do we need to increment here? Won't it get a new id when you do a new upload?

fallaciousreasoning · 2026-08-28T06:18:18Z

+    }
+
+    if (!this.isConnected || uploadAttemptId !== this.uploadAttemptId_) {
+      URL.revokeObjectURL(previewUrl)


super optional, but if you wanted you could play around with using here:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/using

fallaciousreasoning · 2026-08-28T06:22:41Z

  ]
 }

 buildflag_header("custom_profile_image_buildflags") {


I might've lead you astray here. Thinking about it, I think we should have a

//brave/ui/webui/custom_profile_image/buildflags/buildflags.gni for declaring the buildflags and a BUILD.gn in that folder that does this.

Can happen in a separate PR - sorry about that.

bsclifton

Strings LGTM! 😄

chrislacy added 4 commits August 27, 2026 05:28

Add shared Lit custom profile image row

6b2815d

- handle image validation, preview replacement, and object URL cleanup - keep strings and feature state owned by consumers - package the row only when custom profile image WebUI support is enabled

Integrate custom profile images into Manage Profile

ccc53cf

- insert the feature-gated row between the theme and avatar pickers - supply labels through generated BraveSettingsStrings - cover feature state, image selection, and preview lifecycle in Settings tests

Show upload tooltip on empty custom image preview

5bc1f96

- reuse the upload label as hover text for the icon button - verify the tooltip matches its accessible name in Settings tests

Formatting fixes

08c04d5

chrislacy self-assigned this Aug 27, 2026

chrislacy added CI/skip-android Do not run CI builds for Android CI/skip-ios Do not run CI builds for iOS CI/skip-origin Do not run CI builds for Origin labels Aug 27, 2026

github-actions Bot added the CI/storybook-url Deploy storybook and provide a unique URL for each build label Aug 27, 2026

Cleanup strings

2c0fb3b

- add custom profile image replace tooltip - remove unused custom profile image preview label

chrislacy marked this pull request as ready for review August 27, 2026 02:47

chrislacy requested review from a team as code owners August 27, 2026 02:47

fallaciousreasoning reviewed Aug 27, 2026

View reviewed changes

chrislacy and others added 9 commits August 27, 2026 15:24

Always configure "customProfileImageEnabled"

81d1de2

Co-authored-by: Jay Harris <jay.harris@outlook.co.nz>

Simplify code

5fc556a

Compare the two section parents directly, then use the shared parent to insert the custom image section.

Address #include nits

108a893

Remove redundant tooltip fallbacks

b3fbb47

Presubmit fixes

3dc3038

Use Nala styling

d1b047e

Replace Chromium styling usage with corresponding Leo items.

Derive custom image row state

cf601b2

Compute the row state from `localPreviewUrl_` so upload/removal paths don't maintain duplicate state.

Hide custom profile image row title with CSS

507e4a3

- keep the title in the DOM for `aria-labelledby` - let the reflected hide-title attribute control its layout and visibility

Improve string definitions

c55e55b

- resolve labels and tooltips in Lit template so consumers don't need localization-only properties - update Settings test to verify rendered localized content

chrislacy requested a review from fallaciousreasoning August 27, 2026 21:02

chrislacy added 4 commits August 28, 2026 12:58

Use Polymer helper for custom profile image section

2ff6d48

Remove unneeded early exit check

3d6ab9c

Simplify people_page_manage_profile_test.ts

71007c3

Derive custom profile image row state from local preview

60f2750

- use reactive `localPreviewUrl_` as the single source of truth - remove the public state API and unused `state-*` class - centralize dynamic upload/replace strings

chrislacy added 3 commits August 28, 2026 13:21

Use supported Leo styles for custom profile image row

f9d49e3

- share preview styles through `#preview` - size the selected indicator with `--leo-icon-size` - remove unused button-specific styling

Use template i18n for selected profile image label

9f86316

- add the Lit template replacement markers - replace the static selected-preview label with `$i18n{...}`

Revert "Use template i18n for selected profile image label"

42727e5

This reverts commit 9f86316.

fallaciousreasoning approved these changes Aug 28, 2026

View reviewed changes

chrislacy changed the title ~~Desktop/custom profile image/manage profile integration~~ [Desktop] Custom profile image manageProfile integration Aug 28, 2026

bsclifton approved these changes Aug 28, 2026

View reviewed changes

Uh oh!

Conversation

chrislacy commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Screenshots

String Usage

Uh oh!

github-actions Bot commented Aug 27, 2026

📋 Code Owners Summary

Owners and Their Files

@brave/string-reviewers-team — 2 file(s)

@brave/chromium-src-web-reviewers — 1 file(s)

Uh oh!

fallaciousreasoning left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

brave-builds commented Aug 27, 2026

Uh oh!

chrislacy commented Aug 28, 2026

Uh oh!

chrislacy commented Aug 28, 2026

Uh oh!

fallaciousreasoning left a comment

Choose a reason for hiding this comment

Uh oh!

fallaciousreasoning Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fallaciousreasoning Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

fallaciousreasoning Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

fallaciousreasoning Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

fallaciousreasoning Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

fallaciousreasoning Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

fallaciousreasoning Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

fallaciousreasoning Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

bsclifton left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

chrislacy commented Aug 27, 2026 •

edited

Loading

`@brave/string-reviewers-team` — 2 file(s)

`@brave/chromium-src-web-reviewers` — 1 file(s)

fallaciousreasoning Aug 28, 2026 •

edited

Loading