|
|
Chromium Code Reviews|
Created:
4 years, 4 months ago by benwells Modified:
4 years, 4 months ago CC:
chromium-reviews, tfarina, markusheintz_, msramek+watch_chromium.org, raymes+watch_chromium.org, chrome-apps-syd-reviews_chromium.org, felt Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionRemove default action in permission bubbles
This prevents accidental acceptance of permissions, and also works around websites timing permission requests in a way to always get the permission.
BUG=619429
Committed: https://crrev.com/914c24f03eafaf4043867923166e6dc78f41437f
Cr-Commit-Position: refs/heads/master@{#410004}
Patch Set 1 #Patch Set 2 : No default #
Total comments: 3
Messages
Total messages: 22 (14 generated)
The CQ bit was checked by benwells@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromeos_amd64-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...) linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by benwells@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
benwells@chromium.org changed reviewers: + mgiuca@chromium.org
mgiuca to review felt on cc fyi
Description was changed from ========== Changed default action in permission bubbles to be to deny permissions. This prevents accidental acceptance of permissions, and also works around prevents websites timing permission requests in a way to always get the permission. BUG=619429 ========== to ========== Remove default action in permission bubbles This prevents accidental acceptance of permissions, and also works around websites timing permission requests in a way to always get the permission. BUG=619429 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
felt@chromium.org changed reviewers: + felt@chromium.org
https://codereview.chromium.org/2206903002/diff/20001/chrome/browser/ui/views... File chrome/browser/ui/views/website_settings/permission_prompt_impl.cc (right): https://codereview.chromium.org/2206903002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/website_settings/permission_prompt_impl.cc:312: return ui::DIALOG_BUTTON_NONE; Does this have the side effect of changing the button order?
https://codereview.chromium.org/2206903002/diff/20001/chrome/browser/ui/views... File chrome/browser/ui/views/website_settings/permission_prompt_impl.cc (right): https://codereview.chromium.org/2206903002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/website_settings/permission_prompt_impl.cc:312: return ui::DIALOG_BUTTON_NONE; On 2016/08/04 00:17:29, felt wrote: > Does this have the side effect of changing the button order? No, button ordering is controlled here: https://cs.chromium.org/chromium/src/ui/views/window/dialog_client_view.cc?rc... and isn't based on the default button.
lgtm https://codereview.chromium.org/2206903002/diff/20001/chrome/browser/ui/views... File chrome/browser/ui/views/website_settings/permission_prompt_impl.cc (right): https://codereview.chromium.org/2206903002/diff/20001/chrome/browser/ui/views... chrome/browser/ui/views/website_settings/permission_prompt_impl.cc:312: return ui::DIALOG_BUTTON_NONE; On 2016/08/04 05:09:00, benwells wrote: > On 2016/08/04 00:17:29, felt wrote: > > Does this have the side effect of changing the button order? > > No, button ordering is controlled here: > https://cs.chromium.org/chromium/src/ui/views/window/dialog_client_view.cc?rc... > and isn't based on the default button. oh!! that does have the side effect of making this much simpler than i thought.
lgtm
The CQ bit was checked by benwells@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Remove default action in permission bubbles This prevents accidental acceptance of permissions, and also works around websites timing permission requests in a way to always get the permission. BUG=619429 ========== to ========== Remove default action in permission bubbles This prevents accidental acceptance of permissions, and also works around websites timing permission requests in a way to always get the permission. BUG=619429 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Remove default action in permission bubbles This prevents accidental acceptance of permissions, and also works around websites timing permission requests in a way to always get the permission. BUG=619429 ========== to ========== Remove default action in permission bubbles This prevents accidental acceptance of permissions, and also works around websites timing permission requests in a way to always get the permission. BUG=619429 Committed: https://crrev.com/914c24f03eafaf4043867923166e6dc78f41437f Cr-Commit-Position: refs/heads/master@{#410004} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/914c24f03eafaf4043867923166e6dc78f41437f Cr-Commit-Position: refs/heads/master@{#410004} |
