From 006eb5e1914c9d737c5dc88a328c7f4b64531f2c Mon Sep 17 00:00:00 2001 From: Ravjit Singh Uppal Date: Tue, 3 Nov 2015 15:37:56 +0100 Subject: [PATCH 1/3] moved the deleting logic to TabsManager --- .../lightning/activity/BrowserActivity.java | 56 ++++++++----------- .../lightning/activity/TabsManager.java | 29 +++++++++- 2 files changed, 50 insertions(+), 35 deletions(-) diff --git a/app/src/main/java/acr/browser/lightning/activity/BrowserActivity.java b/app/src/main/java/acr/browser/lightning/activity/BrowserActivity.java index 78d888c..136b843 100644 --- a/app/src/main/java/acr/browser/lightning/activity/BrowserActivity.java +++ b/app/src/main/java/acr/browser/lightning/activity/BrowserActivity.java @@ -816,8 +816,14 @@ public abstract class BrowserActivity extends ThemableBrowserActivity implements */ private synchronized void showTab(final int position) { final LightningView currentView = mTabsManager.getCurrentTab(); - final WebView currentWebView = currentView != null ? currentView.getWebView() : null; final LightningView newView = mTabsManager.switchToTab(position); + switchTabs(currentView, newView); + } + + // This is commodity to breack the flow between regular tab management and the CLIQZ's search + // interface. + private void switchTabs(final LightningView currentView, final LightningView newView) { + final WebView currentWebView = currentView != null ? currentView.getWebView() : null; final WebView newWebView = newView != null ? newView.getWebView() : null; if (newView == null || newWebView == null) { return; @@ -1000,14 +1006,11 @@ public abstract class BrowserActivity extends ThemableBrowserActivity implements private synchronized void deleteTab(int position) { final LightningView tabToDelete = mTabsManager.getTabAtPosition(position); - final LightningView currentTab = mTabsManager.getCurrentTab(); if (tabToDelete == null) { return; } - int current = mTabsManager.positionOf(currentTab); - if (!UrlUtils.isSpecialUrl(tabToDelete.getUrl()) && !isIncognito()) { mPreferences.setSavedUrl(tabToDelete.getUrl()); } @@ -1016,37 +1019,22 @@ public abstract class BrowserActivity extends ThemableBrowserActivity implements if (isShown) { mBrowserFrame.setBackgroundColor(mBackgroundColor); } - if (current > position) { - mTabsManager.deleteTab(position); - mEventBus.post(new BrowserEvents.TabsChanged()); - } else if (mTabsManager.size() > position + 1) { - if (current == position) { - showTab(position + 1); - mTabsManager.deleteTab(position); - mEventBus.post(new BrowserEvents.TabsChanged()); - } else { - mTabsManager.deleteTab(position); - } - } else if (mTabsManager.size() > 1) { - if (current == position) { - showTab(position - 1); - mTabsManager.deleteTab(position); - mEventBus.post(new BrowserEvents.TabsChanged()); - } else { - mTabsManager.deleteTab(position); - } - } else { - if (currentTab != null && (UrlUtils.isSpecialUrl(currentTab.getUrl()) - || currentTab.getUrl().equals(mHomepage))) { - closeActivity(); - } else { - mTabsManager.deleteTab(position); - performExitCleanUp(); - tabToDelete.pauseTimers(); - mEventBus.post(new BrowserEvents.TabsChanged()); - finish(); - } + final LightningView currentTab = mTabsManager.getCurrentTab(); + mTabsManager.deleteTab(position); + final LightningView afterTab = mTabsManager.getCurrentTab(); + if (afterTab == null) { +// if (currentTab != null && (UrlUtils.isSpecialUrl(currentTab.getUrl()) +// || currentTab.getUrl().equals(mPreferenceManager.getHomepage()))) { +// closeActivity(); +// } else { + performExitCleanUp(); + finish(); +// } + } else if (afterTab != currentTab) { + switchTabs(currentTab, afterTab); + currentTab.pauseTimers(); } + mEventBus.post(new BrowserEvents.TabsChanged()); if (shouldClose) { diff --git a/app/src/main/java/acr/browser/lightning/activity/TabsManager.java b/app/src/main/java/acr/browser/lightning/activity/TabsManager.java index b4be5a3..6f14bb8 100644 --- a/app/src/main/java/acr/browser/lightning/activity/TabsManager.java +++ b/app/src/main/java/acr/browser/lightning/activity/TabsManager.java @@ -9,6 +9,8 @@ import android.support.v7.app.AlertDialog; import android.util.Log; import android.webkit.WebView; +import com.squareup.otto.Bus; + import java.util.ArrayList; import java.util.List; @@ -16,8 +18,10 @@ import javax.inject.Inject; import javax.inject.Singleton; import acr.browser.lightning.R; +import acr.browser.lightning.bus.BrowserEvents; import acr.browser.lightning.constant.Constants; import acr.browser.lightning.preference.PreferenceManager; +import acr.browser.lightning.utils.UrlUtils; import acr.browser.lightning.utils.Utils; import acr.browser.lightning.view.LightningView; @@ -35,6 +39,9 @@ public class TabsManager { @Inject PreferenceManager mPreferenceManager; + @Inject + Bus mEventBus; + @Inject public TabsManager() {} @@ -182,7 +189,7 @@ public class TabsManager { * @return The removed tab reference or null */ @Nullable - public synchronized LightningView deleteTab(final int position) { + public synchronized LightningView removeTab(final int position) { if (position >= mWebViewList.size()) { return null; } @@ -191,9 +198,29 @@ public class TabsManager { mCurrentTab = null; } tab.onDestroy(); + Log.d(Constants.TAG, tab.toString()); return tab; } + public synchronized void deleteTab(int position) { + final LightningView currentTab = getCurrentTab(); + int current = positionOf(currentTab); + + if (current == position) { + if (size() == 1) { + mCurrentTab = null; + } else if (current < size() - 1 ) { + // There is another tab after this one + mCurrentTab = getTabAtPosition(current + 1); + } else { + mCurrentTab = getTabAtPosition(current - 1); + } + removeTab(current); + } else { + removeTab(position); + } + } + /** * Return the position of the given tab. * From cc75ba1bc7640a70af7e0f8f738ab9e204a312e8 Mon Sep 17 00:00:00 2001 From: Ravjit Singh Uppal Date: Tue, 3 Nov 2015 15:45:44 +0100 Subject: [PATCH 2/3] Changed the scope of removeTab to private --- .../main/java/acr/browser/lightning/activity/TabsManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/acr/browser/lightning/activity/TabsManager.java b/app/src/main/java/acr/browser/lightning/activity/TabsManager.java index 6f14bb8..1d5cdfd 100644 --- a/app/src/main/java/acr/browser/lightning/activity/TabsManager.java +++ b/app/src/main/java/acr/browser/lightning/activity/TabsManager.java @@ -189,7 +189,7 @@ public class TabsManager { * @return The removed tab reference or null */ @Nullable - public synchronized LightningView removeTab(final int position) { + private synchronized LightningView removeTab(final int position) { if (position >= mWebViewList.size()) { return null; } From 6f914e9e17bf64e0c0d110c31b1e27f74d37c878 Mon Sep 17 00:00:00 2001 From: Stefano Pacifici Date: Wed, 4 Nov 2015 14:21:44 +0100 Subject: [PATCH 3/3] Better handling of bookmarks, some responsability moved back to BrowserActivity --- .../lightning/activity/BrowserActivity.java | 67 ++++++++++++------- .../browser/lightning/bus/BookmarkEvents.java | 15 +---- .../browser/lightning/bus/BrowserEvents.java | 9 ++- .../lightning/fragment/BookmarksFragment.java | 21 ++---- 4 files changed, 56 insertions(+), 56 deletions(-) diff --git a/app/src/main/java/acr/browser/lightning/activity/BrowserActivity.java b/app/src/main/java/acr/browser/lightning/activity/BrowserActivity.java index 136b843..09189df 100644 --- a/app/src/main/java/acr/browser/lightning/activity/BrowserActivity.java +++ b/app/src/main/java/acr/browser/lightning/activity/BrowserActivity.java @@ -100,6 +100,7 @@ import acr.browser.lightning.constant.HistoryPage; import acr.browser.lightning.controller.UIController; import acr.browser.lightning.database.BookmarkManager; import acr.browser.lightning.database.HistoryDatabase; +import acr.browser.lightning.database.HistoryItem; import acr.browser.lightning.dialog.LightningDialogBuilder; import acr.browser.lightning.fragment.BookmarksFragment; import acr.browser.lightning.fragment.TabsFragment; @@ -666,6 +667,7 @@ public abstract class BrowserActivity extends ThemableBrowserActivity implements @Override public boolean onOptionsItemSelected(MenuItem item) { final LightningView currentView = mTabsManager.getCurrentTab(); + final String currentUrl = currentView != null ? currentView.getUrl() : null; // Handle action buttons switch (item.getItemId()) { case android.R.id.home: @@ -691,11 +693,11 @@ public abstract class BrowserActivity extends ThemableBrowserActivity implements overridePendingTransition(R.anim.slide_up_in, R.anim.fade_out_scale); return true; case R.id.action_share: - if (currentView != null && !UrlUtils.isSpecialUrl(currentView.getUrl())) { + if (currentUrl != null && !UrlUtils.isSpecialUrl(currentUrl)) { Intent shareIntent = new Intent(Intent.ACTION_SEND); shareIntent.setType("text/plain"); shareIntent.putExtra(Intent.EXTRA_SUBJECT, currentView.getTitle()); - shareIntent.putExtra(Intent.EXTRA_TEXT, currentView.getUrl()); + shareIntent.putExtra(Intent.EXTRA_TEXT, currentUrl); startActivity(Intent.createChooser(shareIntent, getResources().getString(R.string.dialog_title_share))); } return true; @@ -703,9 +705,9 @@ public abstract class BrowserActivity extends ThemableBrowserActivity implements openBookmarks(); return true; case R.id.action_copy: - if (currentView != null && !UrlUtils.isSpecialUrl(currentView.getUrl())) { + if (currentUrl != null && !UrlUtils.isSpecialUrl(currentUrl)) { ClipboardManager clipboard = (ClipboardManager) getSystemService(CLIPBOARD_SERVICE); - ClipData clip = ClipData.newPlainText("label", currentView.getUrl()); + ClipData clip = ClipData.newPlainText("label", currentUrl); clipboard.setPrimaryClip(clip); Utils.showSnackbar(this, R.string.message_link_copied); } @@ -717,18 +719,17 @@ public abstract class BrowserActivity extends ThemableBrowserActivity implements openHistory(); return true; case R.id.action_add_bookmark: - if (currentView != null && !UrlUtils.isSpecialUrl(currentView.getUrl())) { - mEventBus.post(new BrowserEvents.AddBookmark(currentView.getTitle(), - currentView.getUrl())); + if (currentUrl != null && !UrlUtils.isSpecialUrl(currentUrl)) { + addBookmark(currentView.getTitle(), currentUrl); } return true; case R.id.action_find: findInPage(); return true; case R.id.action_reading_mode: - if (currentView != null) { + if (currentUrl != null) { Intent read = new Intent(this, ReadingActivity.class); - read.putExtra(Constants.LOAD_READING_URL, currentView.getUrl()); + read.putExtra(Constants.LOAD_READING_URL, currentUrl); startActivity(read); } return true; @@ -737,6 +738,27 @@ public abstract class BrowserActivity extends ThemableBrowserActivity implements } } + // By using a manager, adds a bookmark and notifies third parties about that + private void addBookmark(final String title, final String url) { + final HistoryItem item = !mBookmarkManager.isBookmark(url) + ? new HistoryItem(url, title) + : null; + if (item != null && mBookmarkManager.addBookmark(item)) { + mSearchAdapter.refreshBookmarks(); + mEventBus.post(new BrowserEvents.BookmarkAdded(title, url)); + } + } + + private void deleteBookmark(final String title, final String url) { + final HistoryItem item = mBookmarkManager.isBookmark(url) + ? new HistoryItem(url, title) + : null; + if (item != null && mBookmarkManager.deleteBookmark(item)) { + mSearchAdapter.refreshBookmarks(); + mEventBus.post(new BrowserEvents.CurrentPageUrl(url)); + } + } + /** * method that shows a dialog asking what string the user wishes to search * for. It highlights the text entered. @@ -1994,29 +2016,26 @@ public abstract class BrowserActivity extends ThemableBrowserActivity implements } /** - * When receive a {@link acr.browser.lightning.bus.BookmarkEvents.WantToBookmarkCurrentPage} + * When receive a {@link BookmarkEvents.ToggleBookmarkForCurrentPage} * message this receiver answer firing the - * {@link acr.browser.lightning.bus.BrowserEvents.AddBookmark} message + * {@link BrowserEvents.BookmarkAdded} message * * @param event an event that the user wishes to bookmark the current page */ @Subscribe - public void bookmarkCurrentPage(final BookmarkEvents.WantToBookmarkCurrentPage event) { + public void bookmarkCurrentPage(final BookmarkEvents.ToggleBookmarkForCurrentPage event) { final LightningView currentTab = mTabsManager.getCurrentTab(); - if (currentTab != null) { - mEventBus.post(new BrowserEvents.AddBookmark(currentTab.getTitle(), currentTab.getUrl())); + final String url = currentTab != null ? currentTab.getUrl() : null; + final String title = currentTab != null ? currentTab.getTitle() : null; + if (url == null) { + return; } - } - /** - * This message is received when a bookmark was added by the - * {@link acr.browser.lightning.fragment.BookmarksFragment} - * - * @param event the event that a bookmark has been added - */ - @Subscribe - public void bookmarkAdded(final BookmarkEvents.Added event) { - mSearchAdapter.refreshBookmarks(); + if (!mBookmarkManager.isBookmark(url)) { + addBookmark(title, url); + } else { + deleteBookmark(title, url); + } } /** diff --git a/app/src/main/java/acr/browser/lightning/bus/BookmarkEvents.java b/app/src/main/java/acr/browser/lightning/bus/BookmarkEvents.java index 1bff1cb..4306ca8 100644 --- a/app/src/main/java/acr/browser/lightning/bus/BookmarkEvents.java +++ b/app/src/main/java/acr/browser/lightning/bus/BookmarkEvents.java @@ -23,20 +23,9 @@ public final class BookmarkEvents { } /** - * The user ask to bookmark the currently displayed page + * The user ask to add/del a bookmark to the currently displayed page */ - public static class WantToBookmarkCurrentPage { - } - - /** - * The bookmark was added - */ - public static class Added { - public final HistoryItem item; - - public Added(final HistoryItem item) { - this.item = item; - } + public static class ToggleBookmarkForCurrentPage { } /** diff --git a/app/src/main/java/acr/browser/lightning/bus/BrowserEvents.java b/app/src/main/java/acr/browser/lightning/bus/BrowserEvents.java index 478dbce..9f2a0cd 100644 --- a/app/src/main/java/acr/browser/lightning/bus/BrowserEvents.java +++ b/app/src/main/java/acr/browser/lightning/bus/BrowserEvents.java @@ -12,14 +12,13 @@ public final class BrowserEvents { } /** - * Used to reply to the {@link acr.browser.lightning.fragment.BookmarksFragment} message - * {@link acr.browser.lightning.bus.BookmarkEvents.WantToBookmarkCurrentPage}. The interaction - * result is a new bookmark added. + * The {@link acr.browser.lightning.activity.BrowserActivity} signal a new bookmark was added + * (mainly to the {@link acr.browser.lightning.fragment.BookmarksFragment}). */ - public static class AddBookmark { + public static class BookmarkAdded { public final String title, url; - public AddBookmark(final String title, final String url) { + public BookmarkAdded(final String title, final String url) { this.title = title; this.url = url; } diff --git a/app/src/main/java/acr/browser/lightning/fragment/BookmarksFragment.java b/app/src/main/java/acr/browser/lightning/fragment/BookmarksFragment.java index d7584cb..e7ffb88 100644 --- a/app/src/main/java/acr/browser/lightning/fragment/BookmarksFragment.java +++ b/app/src/main/java/acr/browser/lightning/fragment/BookmarksFragment.java @@ -30,7 +30,6 @@ import com.squareup.otto.Bus; import com.squareup.otto.Subscribe; import java.util.ArrayList; -import java.util.Collections; import java.util.List; import javax.inject.Inject; @@ -49,7 +48,6 @@ import acr.browser.lightning.dialog.LightningDialogBuilder; import acr.browser.lightning.preference.PreferenceManager; import acr.browser.lightning.async.ImageDownloadTask; import acr.browser.lightning.utils.ThemeUtils; -import acr.browser.lightning.utils.UrlUtils; import acr.browser.lightning.view.LightningView; /** @@ -192,22 +190,17 @@ public class BookmarksFragment extends Fragment implements View.OnClickListener, } @Subscribe - public void addBookmark(final BrowserEvents.AddBookmark event) { - final HistoryItem item = new HistoryItem(event.url, event.title); - if (!UrlUtils.isSpecialUrl(item.getUrl())) { - if (mBookmarkManager.addBookmark(item)) { - mBookmarks.add(item); - Collections.sort(mBookmarks, new BookmarkManager.SortIgnoreCase()); - mBookmarkAdapter.notifyDataSetChanged(); - mEventBus.post(new BookmarkEvents.Added(item)); - updateBookmarkIndicator(event.url); - } - } + public void addBookmark(final BrowserEvents.BookmarkAdded event) { + updateBookmarkIndicator(event.url); + String folder = mBookmarkManager.getCurrentFolder(); + setBookmarkDataSet(mBookmarkManager.getBookmarksFromFolder(folder, true), false); } @Subscribe public void currentPageInfo(final BrowserEvents.CurrentPageUrl event) { updateBookmarkIndicator(event.url); + String folder = mBookmarkManager.getCurrentFolder(); + setBookmarkDataSet(mBookmarkManager.getBookmarksFromFolder(folder, true), false); } @Subscribe @@ -316,7 +309,7 @@ public class BookmarksFragment extends Fragment implements View.OnClickListener, public void onClick(View v) { switch (v.getId()) { case R.id.action_add_bookmark: - mEventBus.post(new BookmarkEvents.WantToBookmarkCurrentPage()); + mEventBus.post(new BookmarkEvents.ToggleBookmarkForCurrentPage()); break; case R.id.action_reading: LightningView currentTab = mTabsManager.getCurrentTab();