From d24dbb5e54d6e9d92d88024a63810695bc8e8263 Mon Sep 17 00:00:00 2001 From: Craig Russell Date: Tue, 6 Feb 2018 09:58:05 +0000 Subject: [PATCH 1/4] Add long press context menu for images with option to download image --- .../app/browser/BrowserViewModelTest.kt | 46 +++++--- .../browser/WebViewLongPressHandlerTest.kt | 107 ++++++++++++++++++ .../duckduckgo/app/browser/BrowserActivity.kt | 43 +++++-- .../app/browser/BrowserViewModel.kt | 27 ++++- .../app/browser/WebViewLongPressHandler.kt | 65 +++++++++++ .../app/browser/di/BrowserModule.kt | 9 +- .../duckduckgo/app/global/ViewModelFactory.kt | 5 +- app/src/main/res/values/strings.xml | 2 + 8 files changed, 276 insertions(+), 28 deletions(-) create mode 100644 app/src/androidTest/java/com/duckduckgo/app/browser/WebViewLongPressHandlerTest.kt create mode 100644 app/src/main/java/com/duckduckgo/app/browser/WebViewLongPressHandler.kt diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserViewModelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserViewModelTest.kt index 9a90e83ac3c5..f72f108e68c1 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserViewModelTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserViewModelTest.kt @@ -23,6 +23,7 @@ import android.arch.lifecycle.Observer import android.arch.persistence.room.Room import android.net.Uri import android.support.test.InstrumentationRegistry +import android.view.MenuItem import android.view.View import com.duckduckgo.app.autocomplete.api.AutoCompleteApi import com.duckduckgo.app.bookmarks.db.BookmarkEntity @@ -51,7 +52,7 @@ import org.junit.Before import org.junit.Rule import org.junit.Test import org.mockito.* -import org.mockito.ArgumentCaptor.forClass +import org.mockito.ArgumentMatchers.anyString import org.mockito.Mockito.never import org.mockito.Mockito.verify @@ -91,11 +92,18 @@ class BrowserViewModelTest { @Mock private lateinit var bookmarksDao: BookmarksDao + @Mock + private lateinit var mockLongPressHandler: LongPressHandler + + @Mock + private lateinit var mockOmnibarConverter: OmnibarEntryConverter + + @Captor + private lateinit var commandCaptor: ArgumentCaptor + private lateinit var db: AppDatabase private lateinit var appConfigurationDao: AppConfigurationDao - private val mockOmnibarConverter: OmnibarEntryConverter = mock() - private lateinit var testee: BrowserViewModel @Before @@ -117,6 +125,7 @@ class BrowserViewModelTest { autoCompleteApi = mockAutoCompleteApi, appSettingsPreferencesStore = mockSettingsStore, bookmarksDao = bookmarksDao, + longPressHandler = mockLongPressHandler, appConfigurationDao = appConfigurationDao) testee.url.observeForever(mockQueryObserver) @@ -241,10 +250,9 @@ class BrowserViewModelTest { @Test fun whenSharedTextReceivedThenNavigationTriggered() { testee.onSharedTextReceived("http://example.com") - val captor: ArgumentCaptor = forClass(Command::class.java) - verify(mockCommandObserver, times(2)).onChanged(captor.capture()) - assertNotNull(captor.value) - assertTrue(captor.value is Navigate) + verify(mockCommandObserver, times(2)).onChanged(commandCaptor.capture()) + assertNotNull(commandCaptor.value) + assertTrue(commandCaptor.value is Navigate) } @Test @@ -395,10 +403,9 @@ class BrowserViewModelTest { @Test fun whenEnteringNonEmptyQueryThenHideKeyboardCommandIssued() { - val captor = ArgumentCaptor.forClass(BrowserViewModel.Command::class.java) testee.onUserSubmittedQuery("foo") - verify(mockCommandObserver, Mockito.atLeastOnce()).onChanged(captor.capture()) - assertTrue(captor.value == Command.HideKeyboard) + verify(mockCommandObserver, Mockito.atLeastOnce()).onChanged(commandCaptor.capture()) + assertTrue(commandCaptor.value == Command.HideKeyboard) } @Test @@ -410,11 +417,10 @@ class BrowserViewModelTest { @Test fun whenNotifiedEnteringFullScreenThenEnterFullScreenCommandIssued() { - val captor = ArgumentCaptor.forClass(BrowserViewModel.Command::class.java) val stubView = View(InstrumentationRegistry.getTargetContext()) testee.goFullScreen(stubView) - verify(mockCommandObserver, Mockito.atLeastOnce()).onChanged(captor.capture()) - assertTrue(captor.lastValue is Command.ShowFullScreen) + verify(mockCommandObserver, Mockito.atLeastOnce()).onChanged(commandCaptor.capture()) + assertTrue(commandCaptor.lastValue is Command.ShowFullScreen) } @Test @@ -427,4 +433,18 @@ class BrowserViewModelTest { fun whenViewModelInitialisedThenFullScreenFlagIsDisabled() { assertFalse(testee.viewState.value!!.isFullScreen) } + + @Test + fun whenUserSelectsDownloadImageOptionFromContextMenuThenDownloadFileCommandIssued() { + whenever(mockLongPressHandler.userSelectedMenuItem(anyString(), any())) + .thenReturn(LongPressHandler.RequiredAction.DownloadFile("example.com")) + + val mockMenuItem : MenuItem = mock() + testee.userSelectedItemFromLongPressMenu("example.com", mockMenuItem) + verify(mockCommandObserver, Mockito.atLeastOnce()).onChanged(commandCaptor.capture()) + assertTrue(commandCaptor.lastValue is Command.DownloadFile) + + val lastCommand = commandCaptor.lastValue as Command.DownloadFile + assertEquals("example.com", lastCommand.url) + } } diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewLongPressHandlerTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewLongPressHandlerTest.kt new file mode 100644 index 000000000000..5c97fb0e657a --- /dev/null +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/WebViewLongPressHandlerTest.kt @@ -0,0 +1,107 @@ +/* + * Copyright (c) 2018 DuckDuckGo + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.duckduckgo.app.browser + +import android.view.ContextMenu +import android.view.MenuItem +import android.webkit.WebView.HitTestResult +import com.nhaarman.mockito_kotlin.eq +import com.nhaarman.mockito_kotlin.never +import com.nhaarman.mockito_kotlin.verify +import com.nhaarman.mockito_kotlin.whenever +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.mockito.ArgumentMatchers.anyInt +import org.mockito.ArgumentMatchers.anyString +import org.mockito.Mock +import org.mockito.MockitoAnnotations + +class WebViewLongPressHandlerTest { + + private lateinit var testee: WebViewLongPressHandler + + @Mock + private lateinit var mockMenu: ContextMenu + + @Mock + private lateinit var mockMenuItem: MenuItem + + @Before + fun setup() { + MockitoAnnotations.initMocks(this) + testee = WebViewLongPressHandler() + } + + @Test + fun whenLongPressedWithImageTypeThenImageOptionsHeaderAddedToMenu() { + testee.handleLongPress(HitTestResult.IMAGE_TYPE, mockMenu) + verify(mockMenu).setHeaderTitle(R.string.imageOptions) + } + + @Test + fun whenLongPressedWithAnchorImageTypeThenImageOptionsHeaderAddedToMenu() { + testee.handleLongPress(HitTestResult.SRC_IMAGE_ANCHOR_TYPE, mockMenu) + verify(mockMenu).setHeaderTitle(R.string.imageOptions) + } + + @Test + fun whenLongPressedWithImageTypeThenDownloadImageMenuAdded() { + testee.handleLongPress(HitTestResult.IMAGE_TYPE, mockMenu) + verify(mockMenu).add(anyInt(), eq(WebViewLongPressHandler.CONTEXT_MENU_ID_DOWNLOAD_IMAGE), anyInt(), eq(R.string.downloadImage)) + } + + @Test + fun whenLongPressedWithAnchorImageTypeThenDownloadImageMenuAdded() { + testee.handleLongPress(HitTestResult.SRC_IMAGE_ANCHOR_TYPE, mockMenu) + verify(mockMenu).add(anyInt(), eq(WebViewLongPressHandler.CONTEXT_MENU_ID_DOWNLOAD_IMAGE), anyInt(), eq(R.string.downloadImage)) + } + + @Test + fun whenLongPressedWithOtherImageTypeThenMenuNotAltered() { + testee.handleLongPress(HitTestResult.UNKNOWN_TYPE, mockMenu) + verify(mockMenu, never()).setHeaderTitle(anyString()) + verify(mockMenu, never()).setHeaderTitle(anyInt()) + verify(mockMenu, never()).add(anyInt()) + verify(mockMenu, never()).add(anyString()) + verify(mockMenu, never()).add(anyInt(), anyInt(), anyInt(), anyInt()) + verify(mockMenu, never()).add(anyInt(), anyInt(), anyInt(), anyString()) + } + + @Test + fun whenUserSelectedDownloadImageOptionThenActionIsDownloadFileActionRequired() { + whenever(mockMenuItem.itemId).thenReturn(WebViewLongPressHandler.CONTEXT_MENU_ID_DOWNLOAD_IMAGE) + val action = testee.userSelectedMenuItem("example.com", mockMenuItem) + assertTrue(action is LongPressHandler.RequiredAction.DownloadFile) + } + + @Test + fun whenUserSelectedDownloadImageOptionThenDownloadFileWithCorrectUrlReturned() { + whenever(mockMenuItem.itemId).thenReturn(WebViewLongPressHandler.CONTEXT_MENU_ID_DOWNLOAD_IMAGE) + val action = testee.userSelectedMenuItem("example.com", mockMenuItem) as LongPressHandler.RequiredAction.DownloadFile + assertEquals("example.com", action.url) + } + + @Test + fun whenUserSelectedUnknownOptionThenNoActionRequiredReturned() { + val unknownMenuId = 123 + whenever(mockMenuItem.itemId).thenReturn(unknownMenuId) + val action = testee.userSelectedMenuItem("example.com", mockMenuItem) + assertTrue(action == LongPressHandler.RequiredAction.None) + } +} \ No newline at end of file diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt index ec7a1add0fc4..329177359f27 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt @@ -28,11 +28,8 @@ import android.os.Build import android.os.Bundle import android.support.v7.widget.LinearLayoutManager import android.text.Editable +import android.view.* import android.view.KeyEvent.KEYCODE_ENTER -import android.view.Menu -import android.view.MenuItem -import android.view.View -import android.view.ViewGroup import android.view.ViewGroup.LayoutParams.MATCH_PARENT import android.view.inputmethod.EditorInfo.IME_ACTION_DONE import android.webkit.CookieManager @@ -178,6 +175,9 @@ class BrowserActivity : DuckDuckGoActivity(), BookmarkDialogCreationListener { is Command.ShowFullScreen -> { webViewFullScreenContainer.addView(it.view, ViewGroup.LayoutParams(MATCH_PARENT, MATCH_PARENT)) } + is Command.DownloadFile -> { + downloadFile(it.url) + } } } @@ -352,12 +352,7 @@ class BrowserActivity : DuckDuckGoActivity(), BookmarkDialogCreationListener { } webView.setDownloadListener { url, _, _, _, _ -> - val request = DownloadManager.Request(Uri.parse(url)) - request.allowScanningByMediaScanner() - request.setNotificationVisibility(VISIBILITY_VISIBLE_NOTIFY_COMPLETED) - val manager = getSystemService(Context.DOWNLOAD_SERVICE) as DownloadManager - manager.enqueue(request) - Toast.makeText(applicationContext, getString(R.string.webviewDownload), Toast.LENGTH_LONG).show() + downloadFile(url) } webView.setOnTouchListener { _, _ -> @@ -367,9 +362,20 @@ class BrowserActivity : DuckDuckGoActivity(), BookmarkDialogCreationListener { false } + registerForContextMenu(webView) + viewModel.registerWebViewListener(webViewClient, webChromeClient) } + private fun downloadFile(url: String?) { + val request = DownloadManager.Request(Uri.parse(url)) + request.allowScanningByMediaScanner() + request.setNotificationVisibility(VISIBILITY_VISIBLE_NOTIFY_COMPLETED) + val manager = getSystemService(Context.DOWNLOAD_SERVICE) as DownloadManager + manager.enqueue(request) + Toast.makeText(applicationContext, getString(R.string.webviewDownload), Toast.LENGTH_LONG).show() + } + override fun onSaveInstanceState(bundle: Bundle) { webView.saveState(bundle) super.onSaveInstanceState(bundle) @@ -380,6 +386,23 @@ class BrowserActivity : DuckDuckGoActivity(), BookmarkDialogCreationListener { webView.restoreState(bundle) } + override fun onCreateContextMenu(menu: ContextMenu, view: View, menuInfo: ContextMenu.ContextMenuInfo?) { + webView.hitTestResult?.let { + viewModel.userLongPressedInWebView(it, menu) + } + } + + override fun onContextItemSelected(item: MenuItem): Boolean { + webView.hitTestResult?.let { + var url = it.extra + if(viewModel.userSelectedItemFromLongPressMenu(url, item)) { + return true + } + } + + return super.onContextItemSelected(item) + } + /** * Dummy view captures touches on areas outside of the toolbar, before the WebView is visible */ diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt index 1850a44ca505..8805040d97ac 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt @@ -23,12 +23,16 @@ import android.net.Uri import android.support.annotation.AnyThread import android.support.annotation.VisibleForTesting import android.support.annotation.WorkerThread +import android.view.ContextMenu +import android.view.MenuItem import android.view.View +import android.webkit.WebView import com.duckduckgo.app.autocomplete.api.AutoCompleteApi import com.duckduckgo.app.autocomplete.api.AutoCompleteApi.AutoCompleteResult import com.duckduckgo.app.bookmarks.db.BookmarkEntity import com.duckduckgo.app.bookmarks.db.BookmarksDao import com.duckduckgo.app.browser.BrowserViewModel.Command.Navigate +import com.duckduckgo.app.browser.LongPressHandler.RequiredAction import com.duckduckgo.app.browser.omnibar.OmnibarEntryConverter import com.duckduckgo.app.global.SingleLiveEvent import com.duckduckgo.app.privacymonitor.SiteMonitor @@ -61,6 +65,7 @@ class BrowserViewModel( private val bookmarksDao: BookmarksDao, private val autoCompleteApi: AutoCompleteApi, private val appSettingsPreferencesStore: SettingsDataStore, + private val longPressHandler: LongPressHandler, appConfigurationDao: AppConfigurationDao) : WebViewClientListener, ViewModel() { data class ViewState( @@ -90,6 +95,7 @@ class BrowserViewModel( object HideKeyboard : Command() object ReinitialiseWebView : Command() class ShowFullScreen(val view: View) : Command() + class DownloadFile(val url: String) : Command() } /* Observable data for Activity to subscribe to */ val viewState: MutableLiveData = MutableLiveData() @@ -322,7 +328,7 @@ class BrowserViewModel( fun addBookmark(title: String, url: String) { bookmarksDao.insert(BookmarkEntity(title = title, url = url)) } - + private fun openUrl(url: String) { command.value = Navigate(url) } @@ -330,6 +336,25 @@ class BrowserViewModel( fun onUserSelectedToEditQuery(query: String) { viewState.value = currentViewState().copy(isEditing = false, showAutoCompleteSuggestions = false, omnibarText = query) } + + fun userLongPressedInWebView(target: WebView.HitTestResult, menu: ContextMenu) { + Timber.i("Long pressed on %s, (extra=%s)", target.type, target.extra) + longPressHandler.handleLongPress(target.type, menu) + } + + fun userSelectedItemFromLongPressMenu(longPressTarget: String, item: MenuItem): Boolean { + val requiredAction = longPressHandler.userSelectedMenuItem(longPressTarget, item) + + return when(requiredAction) { + is RequiredAction.DownloadFile -> { + command.value = Command.DownloadFile(requiredAction.url) + true + } + RequiredAction.None-> { + false + } + } + } } diff --git a/app/src/main/java/com/duckduckgo/app/browser/WebViewLongPressHandler.kt b/app/src/main/java/com/duckduckgo/app/browser/WebViewLongPressHandler.kt new file mode 100644 index 000000000000..fb0a412e1579 --- /dev/null +++ b/app/src/main/java/com/duckduckgo/app/browser/WebViewLongPressHandler.kt @@ -0,0 +1,65 @@ +/* + * Copyright (c) 2018 DuckDuckGo + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.duckduckgo.app.browser + +import android.view.ContextMenu +import android.view.MenuItem +import android.webkit.WebView +import com.duckduckgo.app.browser.LongPressHandler.RequiredAction +import com.duckduckgo.app.browser.LongPressHandler.RequiredAction.DownloadFile +import com.duckduckgo.app.browser.LongPressHandler.RequiredAction.None +import timber.log.Timber +import javax.inject.Inject + + +interface LongPressHandler { + fun handleLongPress(longPressTargetType: Int, menu: ContextMenu) + fun userSelectedMenuItem(longPressTarget: String, item: MenuItem): RequiredAction + + sealed class RequiredAction { + object None : RequiredAction() + class DownloadFile(val url: String) : RequiredAction() + } +} + +class WebViewLongPressHandler @Inject constructor() : LongPressHandler { + + override fun handleLongPress(longPressTargetType: Int, menu: ContextMenu) { + when (longPressTargetType) { + WebView.HitTestResult.IMAGE_TYPE, + WebView.HitTestResult.SRC_IMAGE_ANCHOR_TYPE -> { + menu.setHeaderTitle(R.string.imageOptions) + menu.add(0, CONTEXT_MENU_ID_DOWNLOAD_IMAGE, 0, R.string.downloadImage) + } + else -> Timber.v("App does not yet handle target type: %d", longPressTargetType) + } + } + + override fun userSelectedMenuItem(longPressTarget: String, item: MenuItem): RequiredAction { + return when (item.itemId) { + CONTEXT_MENU_ID_DOWNLOAD_IMAGE -> { + return DownloadFile(longPressTarget) + } + else -> None + } + } + + + companion object { + const val CONTEXT_MENU_ID_DOWNLOAD_IMAGE = 1 + } +} \ No newline at end of file diff --git a/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt b/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt index 349d2308249f..7ddd645d57b5 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt @@ -17,9 +17,7 @@ package com.duckduckgo.app.browser.di import android.webkit.CookieManager -import com.duckduckgo.app.browser.DuckDuckGoRequestRewriter -import com.duckduckgo.app.browser.DuckDuckGoUrlDetector -import com.duckduckgo.app.browser.RequestRewriter +import com.duckduckgo.app.browser.* import dagger.Module import dagger.Provides @@ -33,4 +31,9 @@ class BrowserModule { fun duckDuckGoRequestRewriter(urlDetector: DuckDuckGoUrlDetector): RequestRewriter { return DuckDuckGoRequestRewriter(urlDetector) } + + @Provides + fun webViewLongPressHandler(): LongPressHandler { + return WebViewLongPressHandler() + } } \ No newline at end of file diff --git a/app/src/main/java/com/duckduckgo/app/global/ViewModelFactory.kt b/app/src/main/java/com/duckduckgo/app/global/ViewModelFactory.kt index 8cb8f69f469a..64598b679c8a 100644 --- a/app/src/main/java/com/duckduckgo/app/global/ViewModelFactory.kt +++ b/app/src/main/java/com/duckduckgo/app/global/ViewModelFactory.kt @@ -23,6 +23,7 @@ import com.duckduckgo.app.bookmarks.db.BookmarksDao import com.duckduckgo.app.bookmarks.ui.BookmarksViewModel import com.duckduckgo.app.browser.BrowserViewModel import com.duckduckgo.app.browser.DuckDuckGoUrlDetector +import com.duckduckgo.app.browser.LongPressHandler import com.duckduckgo.app.browser.omnibar.QueryUrlConverter import com.duckduckgo.app.onboarding.store.OnboardingStore import com.duckduckgo.app.onboarding.ui.OnboardingViewModel @@ -55,7 +56,8 @@ class ViewModelFactory @Inject constructor( private val networkLeaderboardDao: NetworkLeaderboardDao, private val bookmarksDao: BookmarksDao, private val autoCompleteApi: AutoCompleteApi, - private val appSettingsPreferencesStore: SettingsDataStore + private val appSettingsPreferencesStore: SettingsDataStore, + private val webViewLongPressHandler: LongPressHandler ) : ViewModelProvider.NewInstanceFactory() { override fun create(modelClass: Class) = @@ -83,5 +85,6 @@ class ViewModelFactory @Inject constructor( bookmarksDao = bookmarksDao, appSettingsPreferencesStore = appSettingsPreferencesStore, appConfigurationDao = appConfigurationDao, + longPressHandler = webViewLongPressHandler, autoCompleteApi = autoCompleteApi) } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 4fe0a9551432..a915dac6c9c1 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -24,6 +24,8 @@ Clear search input No compatible app installed Add Bookmark + Download Image + Image Options Edit query before searching From aac670b9b7e008962d2b52de0df82bd731c04af3 Mon Sep 17 00:00:00 2001 From: Craig Russell Date: Tue, 6 Feb 2018 11:00:53 +0000 Subject: [PATCH 2/4] Add extra log statements --- .../com/duckduckgo/app/browser/WebViewRequestInterceptor.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt b/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt index b1fa52b33ab1..6910e2bc9437 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt @@ -20,10 +20,10 @@ import android.support.annotation.WorkerThread import android.webkit.WebResourceRequest import android.webkit.WebResourceResponse import android.webkit.WebView -import com.duckduckgo.app.surrogates.ResourceSurrogates import com.duckduckgo.app.global.isHttp import com.duckduckgo.app.httpsupgrade.HttpsUpgrader import com.duckduckgo.app.privacymonitor.model.TrustedSites +import com.duckduckgo.app.surrogates.ResourceSurrogates import com.duckduckgo.app.trackerdetection.TrackerDetector import com.duckduckgo.app.trackerdetection.model.ResourceType import timber.log.Timber @@ -74,7 +74,7 @@ class WebViewRequestInterceptor @Inject constructor( val surrogate = resourceSurrogates.get(url) if (surrogate.responseAvailable) { - Timber.v("Surrogate found for %s", url) + Timber.d("Surrogate found for %s", url) return WebResourceResponse( surrogate.mimeType, "UTF-8", @@ -82,6 +82,7 @@ class WebViewRequestInterceptor @Inject constructor( ) } + Timber.d("Blocking request %s", url) return WebResourceResponse(null, null, null) } From 4c6caa85ede805b8d102abcb8d4f398fc82656ee Mon Sep 17 00:00:00 2001 From: Craig Russell Date: Wed, 7 Feb 2018 09:52:43 +0000 Subject: [PATCH 3/4] Remove timber-style logging instances from project --- .../com/duckduckgo/app/browser/BrowserActivity.kt | 12 ++++++------ .../com/duckduckgo/app/browser/BrowserViewModel.kt | 2 +- .../app/browser/WebViewLongPressHandler.kt | 2 +- .../app/browser/WebViewRequestInterceptor.kt | 4 ++-- .../app/surrogates/ResourceSurrogateLoader.kt | 4 ++-- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt index 329177359f27..ef7549edbf07 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt @@ -249,7 +249,6 @@ class BrowserActivity : DuckDuckGoActivity(), BookmarkDialogCreationListener { } val immersiveMode = isImmersiveModeEnabled() - Timber.d("Immersive mode %s", if (immersiveMode) "enabled" else "not enabled") when (viewState.isFullScreen) { true -> if (!immersiveMode) goFullScreen() false -> if (immersiveMode) exitFullScreen() @@ -368,9 +367,10 @@ class BrowserActivity : DuckDuckGoActivity(), BookmarkDialogCreationListener { } private fun downloadFile(url: String?) { - val request = DownloadManager.Request(Uri.parse(url)) - request.allowScanningByMediaScanner() - request.setNotificationVisibility(VISIBILITY_VISIBLE_NOTIFY_COMPLETED) + val request = DownloadManager.Request(Uri.parse(url)).apply { + allowScanningByMediaScanner() + setNotificationVisibility(VISIBILITY_VISIBLE_NOTIFY_COMPLETED) + } val manager = getSystemService(Context.DOWNLOAD_SERVICE) as DownloadManager manager.enqueue(request) Toast.makeText(applicationContext, getString(R.string.webviewDownload), Toast.LENGTH_LONG).show() @@ -394,8 +394,8 @@ class BrowserActivity : DuckDuckGoActivity(), BookmarkDialogCreationListener { override fun onContextItemSelected(item: MenuItem): Boolean { webView.hitTestResult?.let { - var url = it.extra - if(viewModel.userSelectedItemFromLongPressMenu(url, item)) { + val url = it.extra + if (viewModel.userSelectedItemFromLongPressMenu(url, item)) { return true } } diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt index 8805040d97ac..7bd669814792 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt @@ -338,7 +338,7 @@ class BrowserViewModel( } fun userLongPressedInWebView(target: WebView.HitTestResult, menu: ContextMenu) { - Timber.i("Long pressed on %s, (extra=%s)", target.type, target.extra) + Timber.i("Long pressed on ${target.type}, (extra=${target.extra})") longPressHandler.handleLongPress(target.type, menu) } diff --git a/app/src/main/java/com/duckduckgo/app/browser/WebViewLongPressHandler.kt b/app/src/main/java/com/duckduckgo/app/browser/WebViewLongPressHandler.kt index fb0a412e1579..14284e85f41d 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/WebViewLongPressHandler.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/WebViewLongPressHandler.kt @@ -45,7 +45,7 @@ class WebViewLongPressHandler @Inject constructor() : LongPressHandler { menu.setHeaderTitle(R.string.imageOptions) menu.add(0, CONTEXT_MENU_ID_DOWNLOAD_IMAGE, 0, R.string.downloadImage) } - else -> Timber.v("App does not yet handle target type: %d", longPressTargetType) + else -> Timber.v("App does not yet handle target type: $longPressTargetType") } } diff --git a/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt b/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt index 6910e2bc9437..c4c024fb6898 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt @@ -74,7 +74,7 @@ class WebViewRequestInterceptor @Inject constructor( val surrogate = resourceSurrogates.get(url) if (surrogate.responseAvailable) { - Timber.d("Surrogate found for %s", url) + Timber.d("Surrogate found for $url") return WebResourceResponse( surrogate.mimeType, "UTF-8", @@ -82,7 +82,7 @@ class WebViewRequestInterceptor @Inject constructor( ) } - Timber.d("Blocking request %s", url) + Timber.d("Blocking request $url") return WebResourceResponse(null, null, null) } diff --git a/app/src/main/java/com/duckduckgo/app/surrogates/ResourceSurrogateLoader.kt b/app/src/main/java/com/duckduckgo/app/surrogates/ResourceSurrogateLoader.kt index f2cd69aff792..fe46be820d3e 100644 --- a/app/src/main/java/com/duckduckgo/app/surrogates/ResourceSurrogateLoader.kt +++ b/app/src/main/java/com/duckduckgo/app/surrogates/ResourceSurrogateLoader.kt @@ -72,7 +72,7 @@ class ResourceSurrogateLoader @Inject constructor( ruleName = this[0] mimeType = this[1] } - Timber.d("Found new surrogate rule: %s - %s", ruleName, mimeType) + Timber.d("Found new surrogate rule: $ruleName - $mimeType") nextLineIsNewRule = false return@forEach } @@ -96,7 +96,7 @@ class ResourceSurrogateLoader @Inject constructor( functionBuilder.append("\n") } - Timber.d("Processed %d surrogates", surrogates.size) + Timber.d("Processed ${surrogates.size} surrogates") return surrogates } } \ No newline at end of file From f4059af083899e367fa770eb117abbd1cb35808c Mon Sep 17 00:00:00 2001 From: Craig Russell Date: Wed, 7 Feb 2018 16:40:37 +0000 Subject: [PATCH 4/4] Add external storage permission so images and PDFs downloaded externally --- .../app/browser/BrowserViewModelTest.kt | 4 +- app/src/main/AndroidManifest.xml | 1 + .../duckduckgo/app/browser/BrowserActivity.kt | 83 ++++++++++++++++--- .../app/browser/BrowserViewModel.kt | 4 +- app/src/main/res/values/strings.xml | 1 + 5 files changed, 79 insertions(+), 14 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserViewModelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserViewModelTest.kt index f72f108e68c1..575ce421c4ba 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserViewModelTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserViewModelTest.kt @@ -442,9 +442,9 @@ class BrowserViewModelTest { val mockMenuItem : MenuItem = mock() testee.userSelectedItemFromLongPressMenu("example.com", mockMenuItem) verify(mockCommandObserver, Mockito.atLeastOnce()).onChanged(commandCaptor.capture()) - assertTrue(commandCaptor.lastValue is Command.DownloadFile) + assertTrue(commandCaptor.lastValue is Command.DownloadImage) - val lastCommand = commandCaptor.lastValue as Command.DownloadFile + val lastCommand = commandCaptor.lastValue as Command.DownloadImage assertEquals("example.com", lastCommand.url) } } diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index df7c1ada6827..24e39572bb7b 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -5,6 +5,7 @@ + { webViewFullScreenContainer.addView(it.view, ViewGroup.LayoutParams(MATCH_PARENT, MATCH_PARENT)) } - is Command.DownloadFile -> { - downloadFile(it.url) + is Command.DownloadImage -> { + pendingFileDownload = PendingFileDownload(it.url, Environment.DIRECTORY_PICTURES) + downloadFileWithPermissionCheck() } } } @@ -359,7 +370,9 @@ class BrowserActivity : DuckDuckGoActivity(), BookmarkDialogCreationListener { } webView.setDownloadListener { url, _, _, _, _ -> - downloadFile(url) + pendingFileDownload = PendingFileDownload(url, Environment.DIRECTORY_DOWNLOADS) + + downloadFileWithPermissionCheck() } webView.setOnTouchListener { _, _ -> @@ -374,14 +387,56 @@ class BrowserActivity : DuckDuckGoActivity(), BookmarkDialogCreationListener { viewModel.registerWebViewListener(webViewClient, webChromeClient) } - private fun downloadFile(url: String?) { - val request = DownloadManager.Request(Uri.parse(url)).apply { - allowScanningByMediaScanner() - setNotificationVisibility(VISIBILITY_VISIBLE_NOTIFY_COMPLETED) + private fun downloadFileWithPermissionCheck() { + if (hasWriteStoragePermission()) { + downloadFile() + } else { + requestStoragePermission() + } + } + + private fun downloadFile() { + val pending = pendingFileDownload + pending?.let { + val uri = Uri.parse(pending.url) + val guessedFileName = URLUtil.guessFileName(pending.url, null, null) + Timber.i("Guessed filename of $guessedFileName for url ${pending.url}") + val request = DownloadManager.Request(uri).apply { + allowScanningByMediaScanner() + setDestinationInExternalPublicDir(pending.directory, guessedFileName) + setNotificationVisibility(VISIBILITY_VISIBLE_NOTIFY_COMPLETED) + } + val manager = getSystemService(Context.DOWNLOAD_SERVICE) as DownloadManager + manager.enqueue(request) + pendingFileDownload = null + Toast.makeText(applicationContext, getString(R.string.webviewDownload), Toast.LENGTH_LONG).show() + } + } + + private fun hasWriteStoragePermission(): Boolean { + return ContextCompat.checkSelfPermission(this, WRITE_EXTERNAL_STORAGE) == PackageManager.PERMISSION_GRANTED + } + + private fun requestStoragePermission() { + ActivityCompat.requestPermissions(this, arrayOf(WRITE_EXTERNAL_STORAGE), PERMISSION_REQUEST_EXTERNAL_STORAGE) + } + + override fun onRequestPermissionsResult( + requestCode: Int, + permissions: Array, + grantResults: IntArray + ) { + when(requestCode) { + PERMISSION_REQUEST_EXTERNAL_STORAGE -> { + if((grantResults.isNotEmpty()) && grantResults[0] == PackageManager.PERMISSION_GRANTED) { + Timber.i("Permission granted") + downloadFile() + } else { + Timber.i("Permission refused") + Snackbar.make(toolbar, R.string.permissionRequiredToDownload, Snackbar.LENGTH_LONG).show() + } + } } - val manager = getSystemService(Context.DOWNLOAD_SERVICE) as DownloadManager - manager.enqueue(request) - Toast.makeText(applicationContext, getString(R.string.webviewDownload), Toast.LENGTH_LONG).show() } override fun onSaveInstanceState(bundle: Bundle) { @@ -527,6 +582,11 @@ class BrowserActivity : DuckDuckGoActivity(), BookmarkDialogCreationListener { } } + private data class PendingFileDownload( + val url: String, + val directory: String + ) + companion object { fun intent(context: Context, queryExtra: String? = null): Intent { @@ -538,6 +598,9 @@ class BrowserActivity : DuckDuckGoActivity(), BookmarkDialogCreationListener { private const val ADD_BOOKMARK_FRAGMENT_TAG = "ADD_BOOKMARK" private const val QUERY_EXTRA = "QUERY_EXTRA" private const val DASHBOARD_REQUEST_CODE = 100 + private const val PERMISSION_REQUEST_EXTERNAL_STORAGE = 200 + + } } \ No newline at end of file diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt index 7bd669814792..b52910f2ac6b 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt @@ -95,7 +95,7 @@ class BrowserViewModel( object HideKeyboard : Command() object ReinitialiseWebView : Command() class ShowFullScreen(val view: View) : Command() - class DownloadFile(val url: String) : Command() + class DownloadImage(val url: String) : Command() } /* Observable data for Activity to subscribe to */ val viewState: MutableLiveData = MutableLiveData() @@ -347,7 +347,7 @@ class BrowserViewModel( return when(requiredAction) { is RequiredAction.DownloadFile -> { - command.value = Command.DownloadFile(requiredAction.url) + command.value = Command.DownloadImage(requiredAction.url) true } RequiredAction.None-> { diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index c2bee179ee4f..d580e440a091 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -26,6 +26,7 @@ Add Bookmark Download Image Image Options + Downloading requires storage permission Edit query before searching