New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 674398 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug

Blocked on:
issue 674389



Sign in to add a comment

Reflect / update DSE geolocation access in Chrome UI

Project Member Reported by benwells@chromium.org, Dec 15 2016

Issue description

When the current origin is the DSE, whether geolocation is available or not is affected by the DSE geolocation setting being introduced in  issue 674389 .

This should be reflected in the UI. More details are in the design doc: https://docs.google.com/document/d/1iNVcrm5xxQf1FdCPwm2VWPoQ2F521imEomaWL7g2m5o/edit#heading=h.32rtctoevu6j

Initial mocks: https://folio.googleplex.com/chrome-ux/mocks/317-GSA-location-permission/120216_Phase1Settings#%2F02_Phase1.png
 
Cc: rolfe@chromium.org
And suggestions to avoid parentheses:
* Allowed - Google is your search engine
* Allowed | Google is your search engine

Comment 2 by rolfe@chromium.org, Jan 13 2017

UI Review decision:

Page Info
Location - Allowed for current search engine

Site Settings
Location access
Allow for current search engine

Thread for reference:
https://groups.google.com/a/google.com/forum/#!topic/chrome-ui-review/vfshV_Uc0dw
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8be4e3a24c695eca7635d9488821b6e3efdcb2f8

commit 8be4e3a24c695eca7635d9488821b6e3efdcb2f8
Author: benwells <benwells@chromium.org>
Date: Fri Jan 13 01:46:49 2017

Update Search Engines settings UI for new search geolocation system.

The new search geolocation system slightly changes the way geolocation
permission work. This change updates the search engines UI to work with
the new system.

BUG= 674398 

Review-Url: https://codereview.chromium.org/2629523003
Cr-Commit-Position: refs/heads/master@{#443448}

[modify] https://crrev.com/8be4e3a24c695eca7635d9488821b6e3efdcb2f8/chrome/android/java/src/org/chromium/chrome/browser/preferences/SearchEngineAdapter.java

Project Member

Comment 4 by bugdroid1@chromium.org, Jan 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7b28e26454b336fce8ad4eb8a5dd2597c0750eaa

commit 7b28e26454b336fce8ad4eb8a5dd2597c0750eaa
Author: benwells <benwells@chromium.org>
Date: Fri Jan 13 01:53:52 2017

Update page info for new search geolocation system.

Geolocation access via the geo API and via the X-Geo header has been
made consistent. As part of this project the UI is being updated to show
the geolocation status for the default search engine in more places.

This change updates the page info popup to show allow / block for the
default search engine.

BUG= 674398 

Review-Url: https://codereview.chromium.org/2633433002
Cr-Commit-Position: refs/heads/master@{#443454}

[modify] https://crrev.com/7b28e26454b336fce8ad4eb8a5dd2597c0750eaa/chrome/android/java/src/org/chromium/chrome/browser/pageinfo/WebsiteSettingsPopup.java
[modify] https://crrev.com/7b28e26454b336fce8ad4eb8a5dd2597c0750eaa/chrome/android/java/strings/android_chrome_strings.grd
[modify] https://crrev.com/7b28e26454b336fce8ad4eb8a5dd2597c0750eaa/chrome/browser/ui/android/page_info/website_settings_popup_android.cc
[modify] https://crrev.com/7b28e26454b336fce8ad4eb8a5dd2597c0750eaa/chrome/browser/ui/android/page_info/website_settings_popup_android.h

Project Member

Comment 5 by bugdroid1@chromium.org, Jan 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f34fbb504afba2fa86f1dd7bbfd90e0134cbe639

commit f34fbb504afba2fa86f1dd7bbfd90e0134cbe639
Author: raymes <raymes@chromium.org>
Date: Sun Jan 22 22:40:19 2017

Refactor SearchGeolocationService to make it testable

This refactors SearchGeolocationService to make it testable. Search engine
related functionality, which cannot easily be mocked out in unittests, is moved
into a delegate class.

A couple of other minor fixes are made:
-An IsOffTheRecord check is made into a DCHECK since the class should never be
constructed for an off the record profile.
-The CCTLD is no longer stored because it was never used.
-An extra DCHECK is added in SetDSEGeolocationSetting to ensure the setting is
only changed when Google is the current search engine.

BUG= 674398 

Review-Url: https://codereview.chromium.org/2637953003
Cr-Commit-Position: refs/heads/master@{#445304}

[modify] https://crrev.com/f34fbb504afba2fa86f1dd7bbfd90e0134cbe639/chrome/browser/android/search_geolocation/search_geolocation_service.cc
[modify] https://crrev.com/f34fbb504afba2fa86f1dd7bbfd90e0134cbe639/chrome/browser/android/search_geolocation/search_geolocation_service.h

Project Member

Comment 7 by bugdroid1@chromium.org, Feb 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/64dcf43897c11b5ddbfd849fe1145a74fe0b0b89

commit 64dcf43897c11b5ddbfd849fe1145a74fe0b0b89
Author: benwells <benwells@chromium.org>
Date: Wed Feb 01 22:46:55 2017

Update the descriptions of new permission string for search geolocation

The desc string for the blocked string was the same as the desc string
for the allowed string. Also the descriptions were slightly out of date and reflected the older strings.

BUG= 674398 

Review-Url: https://codereview.chromium.org/2662093003
Cr-Commit-Position: refs/heads/master@{#447625}

[modify] https://crrev.com/64dcf43897c11b5ddbfd849fe1145a74fe0b0b89/chrome/android/java/strings/android_chrome_strings.grd

Status: Fixed (was: Assigned)

Sign in to add a comment