Add permission for installing language packs [chromium/src : main]

1 view
Skip to first unread message

Evan Liu (Gerrit)

unread,
Aug 28, 2024, 5:03:17 PM8/28/24
to Demetrios Papadopoulos, Florian Jacky, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, devtools...@chromium.org, Luna Lu, Permissions Reviews, Peter Beverloo, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, msrame...@chromium.org, srahim...@chromium.org
Attention needed from Demetrios Papadopoulos and Florian Jacky

Evan Liu added 1 comment

Patchset-level comments
File-level comment, Patchset 10 (Latest):
Evan Liu . resolved

dpapad@ - Please take a look at the changes to the WebUI parts.
fjacky@ - Please take a look at the changes to //components/permissions/*.

Open in Gerrit

Related details

Attention is currently required from:
  • Demetrios Papadopoulos
  • Florian Jacky
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Review
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I7817164754448a0b6b1a65a293efdcd62dcb9477
Gerrit-Change-Number: 5808012
Gerrit-PatchSet: 10
Gerrit-Owner: Evan Liu <ev...@google.com>
Gerrit-Reviewer: Demetrios Papadopoulos <dpa...@chromium.org>
Gerrit-Reviewer: Evan Liu <ev...@google.com>
Gerrit-Reviewer: Florian Jacky <fja...@chromium.org>
Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
Gerrit-CC: Luna Lu <loon...@chromium.org>
Gerrit-CC: Permissions Reviews <permissio...@chromium.org>
Gerrit-CC: Peter Beverloo <pe...@chromium.org>
Gerrit-Attention: Demetrios Papadopoulos <dpa...@chromium.org>
Gerrit-Attention: Florian Jacky <fja...@chromium.org>
Gerrit-Comment-Date: Wed, 28 Aug 2024 21:03:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

Florian Jacky (Gerrit)

unread,
Aug 29, 2024, 5:02:43 AM8/29/24
to Evan Liu, Demetrios Papadopoulos, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, devtools...@chromium.org, Luna Lu, Permissions Reviews, Peter Beverloo, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, msrame...@chromium.org, srahim...@chromium.org
Attention needed from Demetrios Papadopoulos and Evan Liu

Florian Jacky added 2 comments

File components/permissions_strings.grdp
Line 181, Patchset 10 (Latest): Download a language pack
Florian Jacky . unresolved

Is this prompted for each download or can other language packs be downloaded after the permission is allowed? Would it make sense to specify the actual language pack being downloaded?

File components/permissions_strings_grdp/IDS_DOWNLOAD_LANGUAGE_PACK.png
Florian Jacky . unresolved

The png itself shouldn't be checked into the repo. It's uploaded to a server in the process of generating the hash file, so only the .sha1 file needs to be checked in.

Open in Gerrit

Related details

Attention is currently required from:
  • Demetrios Papadopoulos
  • Evan Liu
Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I7817164754448a0b6b1a65a293efdcd62dcb9477
    Gerrit-Change-Number: 5808012
    Gerrit-PatchSet: 10
    Gerrit-Owner: Evan Liu <ev...@google.com>
    Gerrit-Reviewer: Demetrios Papadopoulos <dpa...@chromium.org>
    Gerrit-Reviewer: Evan Liu <ev...@google.com>
    Gerrit-Reviewer: Florian Jacky <fja...@chromium.org>
    Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
    Gerrit-CC: Luna Lu <loon...@chromium.org>
    Gerrit-CC: Permissions Reviews <permissio...@chromium.org>
    Gerrit-CC: Peter Beverloo <pe...@chromium.org>
    Gerrit-Attention: Demetrios Papadopoulos <dpa...@chromium.org>
    Gerrit-Attention: Evan Liu <ev...@google.com>
    Gerrit-Comment-Date: Thu, 29 Aug 2024 09:02:31 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Evan Liu (Gerrit)

    unread,
    Aug 29, 2024, 3:54:02 PM8/29/24
    to Demetrios Papadopoulos, Florian Jacky, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, devtools...@chromium.org, Luna Lu, Permissions Reviews, Peter Beverloo, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, msrame...@chromium.org, srahim...@chromium.org
    Attention needed from Demetrios Papadopoulos and Florian Jacky

    Evan Liu voted and added 2 comments

    Votes added by Evan Liu

    Commit-Queue+1

    2 comments

    File components/permissions_strings.grdp
    Line 181, Patchset 10: Download a language pack
    Florian Jacky . resolved

    Is this prompted for each download or can other language packs be downloaded after the permission is allowed? Would it make sense to specify the actual language pack being downloaded?

    Evan Liu

    The intention is to allow other language packs to be downloaded after this permission is allowed. The exact string is still being reviewed by dwarren@ so this is subject to change. I'll update this CL when I hear back from him (or update in a follow-up CL if that's acceptable).

    File components/permissions_strings_grdp/IDS_DOWNLOAD_LANGUAGE_PACK.png
    File-level comment, Patchset 10:
    Florian Jacky . resolved

    The png itself shouldn't be checked into the repo. It's uploaded to a server in the process of generating the hash file, so only the .sha1 file needs to be checked in.

    Evan Liu

    Oops, nice catch! Removed

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Demetrios Papadopoulos
    • Florian Jacky
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Review
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I7817164754448a0b6b1a65a293efdcd62dcb9477
    Gerrit-Change-Number: 5808012
    Gerrit-PatchSet: 12
    Gerrit-Owner: Evan Liu <ev...@google.com>
    Gerrit-Reviewer: Demetrios Papadopoulos <dpa...@chromium.org>
    Gerrit-Reviewer: Evan Liu <ev...@google.com>
    Gerrit-Reviewer: Florian Jacky <fja...@chromium.org>
    Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
    Gerrit-CC: Luna Lu <loon...@chromium.org>
    Gerrit-CC: Permissions Reviews <permissio...@chromium.org>
    Gerrit-CC: Peter Beverloo <pe...@chromium.org>
    Gerrit-Attention: Florian Jacky <fja...@chromium.org>
    Gerrit-Attention: Demetrios Papadopoulos <dpa...@chromium.org>
    Gerrit-Comment-Date: Thu, 29 Aug 2024 19:53:51 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    Comment-In-Reply-To: Florian Jacky <fja...@chromium.org>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Demetrios Papadopoulos (Gerrit)

    unread,
    Sep 3, 2024, 1:47:04 PM9/3/24
    to Evan Liu, Rainhard Findling, Florian Jacky, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, devtools...@chromium.org, Luna Lu, Permissions Reviews, Peter Beverloo, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, msrame...@chromium.org, srahim...@chromium.org
    Attention needed from Evan Liu, Florian Jacky and Rainhard Findling

    Demetrios Papadopoulos added 1 comment

    Patchset-level comments

    dpapad@ - Please take a look at the changes to the WebUI parts.
    fjacky@ - Please take a look at the changes to //components/permissions/*.

    Demetrios Papadopoulos

    dpapad@ - Please take a look at the changes to the WebUI parts.

    Redirecting site_settings/ and privacy_page/ changes to more specific owners.

    Having said that, is there a way to break up this CL to smaller pieces? I think it would help make it easier to review.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Evan Liu
    • Florian Jacky
    • Rainhard Findling
    Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement is not satisfiedCode-Review
      • requirement is not satisfiedNo-Unresolved-Comments
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: I7817164754448a0b6b1a65a293efdcd62dcb9477
      Gerrit-Change-Number: 5808012
      Gerrit-PatchSet: 13
      Gerrit-Owner: Evan Liu <ev...@google.com>
      Gerrit-Reviewer: Evan Liu <ev...@google.com>
      Gerrit-Reviewer: Florian Jacky <fja...@chromium.org>
      Gerrit-Reviewer: Rainhard Findling <rain...@chromium.org>
      Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
      Gerrit-CC: Demetrios Papadopoulos <dpa...@chromium.org>
      Gerrit-CC: Luna Lu <loon...@chromium.org>
      Gerrit-CC: Permissions Reviews <permissio...@chromium.org>
      Gerrit-CC: Peter Beverloo <pe...@chromium.org>
      Gerrit-Attention: Rainhard Findling <rain...@chromium.org>
      Gerrit-Attention: Florian Jacky <fja...@chromium.org>
      Gerrit-Attention: Evan Liu <ev...@google.com>
      Gerrit-Comment-Date: Tue, 03 Sep 2024 17:46:50 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      Comment-In-Reply-To: Evan Liu <ev...@google.com>
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy

      Rainhard Findling (Gerrit)

      unread,
      Sep 3, 2024, 1:49:15 PM9/3/24
      to Evan Liu, Demetrios Papadopoulos, Florian Jacky, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, devtools...@chromium.org, Luna Lu, Permissions Reviews, Peter Beverloo, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, msrame...@chromium.org, srahim...@chromium.org
      Attention needed from Demetrios Papadopoulos, Evan Liu and Florian Jacky

      Rainhard Findling added 1 comment

      Patchset-level comments
      Evan Liu . unresolved

      dpapad@ - Please take a look at the changes to the WebUI parts.
      fjacky@ - Please take a look at the changes to //components/permissions/*.

      Demetrios Papadopoulos

      dpapad@ - Please take a look at the changes to the WebUI parts.

      Redirecting site_settings/ and privacy_page/ changes to more specific owners.

      Having said that, is there a way to break up this CL to smaller pieces? I think it would help make it easier to review.

      Rainhard Findling

      Having said that, is there a way to break up this CL to smaller pieces? I think it would help make it easier to review.

      +1

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Demetrios Papadopoulos
      • Evan Liu
      • Florian Jacky
      Gerrit-Attention: Demetrios Papadopoulos <dpa...@chromium.org>
      Gerrit-Attention: Florian Jacky <fja...@chromium.org>
      Gerrit-Attention: Evan Liu <ev...@google.com>
      Gerrit-Comment-Date: Tue, 03 Sep 2024 17:49:01 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      Comment-In-Reply-To: Demetrios Papadopoulos <dpa...@chromium.org>
      Comment-In-Reply-To: Evan Liu <ev...@google.com>
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy

      Florian Jacky (Gerrit)

      unread,
      Sep 4, 2024, 5:42:30 AM9/4/24
      to Evan Liu, Rainhard Findling, Demetrios Papadopoulos, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, devtools...@chromium.org, Luna Lu, Permissions Reviews, Peter Beverloo, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, msrame...@chromium.org, srahim...@chromium.org
      Attention needed from Demetrios Papadopoulos and Evan Liu

      Florian Jacky added 1 comment

      File components/permissions_strings.grdp
      Line 181, Patchset 10: Download a language pack
      Florian Jacky . unresolved

      Is this prompted for each download or can other language packs be downloaded after the permission is allowed? Would it make sense to specify the actual language pack being downloaded?

      Evan Liu

      The intention is to allow other language packs to be downloaded after this permission is allowed. The exact string is still being reviewed by dwarren@ so this is subject to change. I'll update this CL when I hear back from him (or update in a follow-up CL if that's acceptable).

      Florian Jacky

      I'm fine with merging a temporary string and finalizing it later before the launch, however the current string seems somewhat misleading. If you want to merge before it is finalized, could it be changed to "Download language packs" for now?

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Demetrios Papadopoulos
      • Evan Liu
      Gerrit-Attention: Evan Liu <ev...@google.com>
      Gerrit-Comment-Date: Wed, 04 Sep 2024 09:42:18 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      Comment-In-Reply-To: Florian Jacky <fja...@chromium.org>
      Comment-In-Reply-To: Evan Liu <ev...@google.com>
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy
      Reply all
      Reply to author
      Forward
      0 new messages