From 65699b67e38a7d6d78088157294ef84251305a7c Mon Sep 17 00:00:00 2001 From: Lucas Rocha Date: Tue, 29 Oct 2013 11:26:25 +0000 Subject: [PATCH 01/24] Bug 925722 - Handle touch events in HomeSearchListView with onTouchEvent (r=sriram) --- mobile/android/base/home/BrowserSearch.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mobile/android/base/home/BrowserSearch.java b/mobile/android/base/home/BrowserSearch.java index 2ca0df7a7eea..9f70430c919d 100644 --- a/mobile/android/base/home/BrowserSearch.java +++ b/mobile/android/base/home/BrowserSearch.java @@ -877,13 +877,13 @@ public class BrowserSearch extends HomeFragment } @Override - public boolean onInterceptTouchEvent(MotionEvent event) { + public boolean onTouchEvent(MotionEvent event) { if (event.getActionMasked() == MotionEvent.ACTION_DOWN) { // Dismiss the soft keyboard. requestFocus(); } - return super.onInterceptTouchEvent(event); + return super.onTouchEvent(event); } } } From 723c1317fc084c7634cc8be057440e4413c32b81 Mon Sep 17 00:00:00 2001 From: Mark Capella Date: Tue, 29 Oct 2013 08:42:37 -0400 Subject: [PATCH 02/24] Bug 906402 - security exception when checking signature of favicon, r=mfinkle --- mobile/android/base/gfx/BitmapUtils.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/mobile/android/base/gfx/BitmapUtils.java b/mobile/android/base/gfx/BitmapUtils.java index cd029adbfd29..905a79c29b12 100644 --- a/mobile/android/base/gfx/BitmapUtils.java +++ b/mobile/android/base/gfx/BitmapUtils.java @@ -61,6 +61,11 @@ public final class BitmapUtils { return GeckoJarReader.getBitmapDrawable(context.getResources(), data); } + // Don't attempt to validate the JAR signature when loading an add-on icon + if (data.startsWith("jar:file")) { + return GeckoJarReader.getBitmapDrawable(context.getResources(), Uri.decode(data)); + } + URL url = new URL(data); InputStream is = (InputStream) url.getContent(); try { From b852bba32d5aa9dd6092527e252ac10bbae45c3d Mon Sep 17 00:00:00 2001 From: Robert Strong Date: Tue, 29 Oct 2013 07:55:01 -0700 Subject: [PATCH 03/24] xpcshell test change only - Bug 932022 - xpcshell test downloadResumeForSameAppVersion.js always leaves behind downloadResumeForSameAppVersion_applyToDir ater it finishes. r=bbondy --- .../unit_aus_update/downloadResumeForSameAppVersion.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/toolkit/mozapps/update/tests/unit_aus_update/downloadResumeForSameAppVersion.js b/toolkit/mozapps/update/tests/unit_aus_update/downloadResumeForSameAppVersion.js index 03827297bc63..e9e5030be378 100644 --- a/toolkit/mozapps/update/tests/unit_aus_update/downloadResumeForSameAppVersion.js +++ b/toolkit/mozapps/update/tests/unit_aus_update/downloadResumeForSameAppVersion.js @@ -30,7 +30,15 @@ function run_test() { } do_check_eq(gUpdateManager.activeUpdate.state, STATE_DOWNLOADING); - do_test_finished(); + // Pause the download and reload the Update Manager with an empty update so + // the Application Update Service doesn't write the update xml files during + // xpcom-shutdown which will leave behind the test directory. + gAUS.pauseDownload(); + writeUpdatesToXMLFile(getLocalUpdatesXMLString(""), true); + writeUpdatesToXMLFile(getLocalUpdatesXMLString(""), false); + reloadUpdateManagerData(); + + do_timeout(TEST_CHECK_TIMEOUT, do_test_finished); } function end_test() { From 0acfb1147acf4eafbc1bd4ec0a4b3be6d4b74b1e Mon Sep 17 00:00:00 2001 From: Michael Comella Date: Thu, 17 Oct 2013 13:32:57 -0700 Subject: [PATCH 04/24] Bug 924480 - Part 1: Make search_container focusable before browser_toolbar. r=bnicholson --- mobile/android/base/BrowserToolbar.java | 1 + .../base/resources/layout/gecko_app.xml | 21 ++++++++++++------- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/mobile/android/base/BrowserToolbar.java b/mobile/android/base/BrowserToolbar.java index b1c88879b6ce..f420da4a4bab 100644 --- a/mobile/android/base/BrowserToolbar.java +++ b/mobile/android/base/BrowserToolbar.java @@ -410,6 +410,7 @@ public class BrowserToolbar extends GeckoRelativeLayout } if (keyCode == KeyEvent.KEYCODE_BACK) { + // Drop the virtual keyboard. clearFocus(); return true; } diff --git a/mobile/android/base/resources/layout/gecko_app.xml b/mobile/android/base/resources/layout/gecko_app.xml index 8d3407933c2a..6b9ca9d8df26 100644 --- a/mobile/android/base/resources/layout/gecko_app.xml +++ b/mobile/android/base/resources/layout/gecko_app.xml @@ -55,20 +55,25 @@ android:layout_alignParentBottom="true"> - + + + - - Date: Wed, 23 Oct 2013 12:15:34 -0700 Subject: [PATCH 05/24] Bug 924480 - Part 1.5: Make HomePager focusable. r=lucasr --- mobile/android/base/home/HomePager.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/mobile/android/base/home/HomePager.java b/mobile/android/base/home/HomePager.java index d07debd36d5c..52512320de4d 100644 --- a/mobile/android/base/home/HomePager.java +++ b/mobile/android/base/home/HomePager.java @@ -96,6 +96,13 @@ public class HomePager extends ViewPager { // This is to keep all 4 pages in memory after they are // selected in the pager. setOffscreenPageLimit(3); + + // We can call HomePager.requestFocus to steal focus from the URL bar and drop the soft + // keyboard. However, if there are no focusable views (e.g. an empty reading list), the + // URL bar will be refocused. Therefore, we make the HomePager container focusable to + // ensure there is always a focusable view. This would ordinarily be done via an XML + // attribute, but it is not working properly. + setFocusableInTouchMode(true); } @Override @@ -324,10 +331,7 @@ public class HomePager extends ViewPager { @Override public boolean onInterceptTouchEvent(MotionEvent event) { if (event.getActionMasked() == MotionEvent.ACTION_DOWN) { - // XXX: Drop the soft keyboard by stealing focus. Note that the HomePager (via XML - // attr) is focusable after its descendants allowing requestFocus to succeed and drop - // the soft keyboard even if there are no other focusable views on the screen (e.g. - // the Reading List is empty). + // Drop the soft keyboard by stealing focus from the URL bar. requestFocus(); } From 701a3f885da81bb798ecbcf525f56f697414f627 Mon Sep 17 00:00:00 2001 From: Michael Comella Date: Fri, 18 Oct 2013 09:40:28 -0700 Subject: [PATCH 06/24] Bug 924480 - Part 2: Disable TalkBack access to Gecko with HomePager displayed. r=eeejay,lucasr --- mobile/android/base/BrowserApp.java | 43 +++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/mobile/android/base/BrowserApp.java b/mobile/android/base/BrowserApp.java index cb9183e68bfd..a81c8fa11e8a 100644 --- a/mobile/android/base/BrowserApp.java +++ b/mobile/android/base/BrowserApp.java @@ -167,6 +167,12 @@ abstract public class BrowserApp extends GeckoApp private BrowserHealthReporter mBrowserHealthReporter; + // The animator used to toggle HomePager visibility has a race where if the HomePager is shown + // (starting the animation), the HomePager is hidden, and the HomePager animation completes, + // both the web content and the HomePager will be hidden. This flag is used to prevent the + // race by determining if the web content should be hidden at the animation's end. + private boolean mHideWebContentOnAnimationEnd = false; + private SiteIdentityPopup mSiteIdentityPopup; public SiteIdentityPopup getSiteIdentityPopup() { @@ -1574,7 +1580,38 @@ abstract public class BrowserApp extends GeckoApp final ViewStub homePagerStub = (ViewStub) findViewById(R.id.home_pager_stub); mHomePager = (HomePager) homePagerStub.inflate(); } + mHomePager.show(getSupportFragmentManager(), page, animator); + + // Hide the web content so it cannot be focused by screen readers. + hideWebContentOnPropertyAnimationEnd(animator); + } + + private void hideWebContentOnPropertyAnimationEnd(final PropertyAnimator animator) { + if (animator == null) { + hideWebContent(); + return; + } + + animator.addPropertyAnimationListener(new PropertyAnimator.PropertyAnimationListener() { + @Override + public void onPropertyAnimationStart() { + mHideWebContentOnAnimationEnd = true; + } + + @Override + public void onPropertyAnimationEnd() { + if (mHideWebContentOnAnimationEnd) { + hideWebContent(); + } + } + }); + } + + private void hideWebContent() { + // The view is set to INVISIBLE, rather than GONE, to avoid + // the additional requestLayout() call. + mLayerView.setVisibility(View.INVISIBLE); } private void hideHomePager() { @@ -1587,6 +1624,12 @@ abstract public class BrowserApp extends GeckoApp return; } + // Prevent race in hiding web content - see declaration for more info. + mHideWebContentOnAnimationEnd = false; + + // Display the previously hidden web content (which prevented screen reader access). + mLayerView.setVisibility(View.VISIBLE); + if (mHomePager != null) { mHomePager.hide(); } From f716edc0abc7835130baf03d9d378c8158f80aa4 Mon Sep 17 00:00:00 2001 From: Michael Comella Date: Mon, 21 Oct 2013 12:11:08 -0700 Subject: [PATCH 07/24] Bug 915918 - Part 1: Select previously selected tab upon editing mode exit. r=lucasr --- mobile/android/base/BrowserApp.java | 31 +++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/mobile/android/base/BrowserApp.java b/mobile/android/base/BrowserApp.java index a81c8fa11e8a..de313f86922e 100644 --- a/mobile/android/base/BrowserApp.java +++ b/mobile/android/base/BrowserApp.java @@ -167,6 +167,9 @@ abstract public class BrowserApp extends GeckoApp private BrowserHealthReporter mBrowserHealthReporter; + // The tab to be selected on editing mode exit. + private Integer mTargetTabForEditingMode = null; + // The animator used to toggle HomePager visibility has a race where if the HomePager is shown // (starting the animation), the HomePager is hidden, and the HomePager animation completes, // both the web content and the HomePager will be hidden. This flag is used to prevent the @@ -487,7 +490,9 @@ abstract public class BrowserApp extends GeckoApp mBrowserToolbar.setOnStopEditingListener(new BrowserToolbar.OnStopEditingListener() { public void onStopEditing() { - // Re-enable doorhanger notifications. + selectTargetTabForEditingMode(); + + // Re-enable doorhanger notifications. They may trigger on the selected tab above. mDoorHangerPopup.enable(); } }); @@ -1311,7 +1316,10 @@ abstract public class BrowserApp extends GeckoApp if (tabs.isSelectedTabId(tabId)) { hideHomePager(); } else { - tabs.selectTab(tabId); + // Set the target tab to null so it does not get selected (on editing + // mode exit) in lieu of the tab we are about to select. + mTargetTabForEditingMode = null; + Tabs.getInstance().selectTab(tabId); } hideBrowserSearch(); @@ -1425,6 +1433,9 @@ abstract public class BrowserApp extends GeckoApp throw new IllegalArgumentException("Cannot handle null URLs in enterEditingMode"); } + final Tab selectedTab = Tabs.getInstance().getSelectedTab(); + mTargetTabForEditingMode = (selectedTab != null ? selectedTab.getId() : null); + final PropertyAnimator animator = new PropertyAnimator(250); animator.setUseHardwareLayer(false); @@ -1537,6 +1548,22 @@ abstract public class BrowserApp extends GeckoApp } } + /** + * Selects the target tab for editing mode. This is expected to be the tab selected on editing + * mode entry, unless it is subsequently overridden. + * + * A background tab may be selected while editing mode is active (e.g. popups), causing the + * new url to load in the newly selected tab. Call this method on editing mode exit to + * mitigate this. + */ + private void selectTargetTabForEditingMode() { + if (mTargetTabForEditingMode != null) { + Tabs.getInstance().selectTab(mTargetTabForEditingMode); + } + + mTargetTabForEditingMode = null; + } + /** * Shows or hides the home pager for the given tab. */ From 770c7a62a5d31b6592eeb95b37363c8a0cb6d3dd Mon Sep 17 00:00:00 2001 From: Michael Comella Date: Wed, 23 Oct 2013 15:18:22 -0700 Subject: [PATCH 08/24] Bug 915918 - Part 1.5: Refactor hiding of BrowserSearch and HomePager into onStopEditing. r=lucasr --- mobile/android/base/BrowserApp.java | 31 +++++++++++------------------ 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/mobile/android/base/BrowserApp.java b/mobile/android/base/BrowserApp.java index de313f86922e..1c1bb03c0185 100644 --- a/mobile/android/base/BrowserApp.java +++ b/mobile/android/base/BrowserApp.java @@ -491,6 +491,8 @@ abstract public class BrowserApp extends GeckoApp mBrowserToolbar.setOnStopEditingListener(new BrowserToolbar.OnStopEditingListener() { public void onStopEditing() { selectTargetTabForEditingMode(); + hideHomePager(); + hideBrowserSearch(); // Re-enable doorhanger notifications. They may trigger on the selected tab above. mDoorHangerPopup.enable(); @@ -1312,17 +1314,11 @@ abstract public class BrowserApp extends GeckoApp return false; } - // If this tab is already selected, just hide the home pager. - if (tabs.isSelectedTabId(tabId)) { - hideHomePager(); - } else { - // Set the target tab to null so it does not get selected (on editing - // mode exit) in lieu of the tab we are about to select. - mTargetTabForEditingMode = null; - Tabs.getInstance().selectTab(tabId); - } + // Set the target tab to null so it does not get selected (on editing + // mode exit) in lieu of the tab we are about to select. + mTargetTabForEditingMode = null; + Tabs.getInstance().selectTab(tabId); - hideBrowserSearch(); mBrowserToolbar.cancelEdit(); return true; @@ -1350,7 +1346,6 @@ abstract public class BrowserApp extends GeckoApp Tabs.getInstance().loadUrl(url, searchEngine, -1, flags); - hideBrowserSearch(); mBrowserToolbar.cancelEdit(); } @@ -1451,8 +1446,6 @@ abstract public class BrowserApp extends GeckoApp } final String url = mBrowserToolbar.commitEdit(); - hideHomePager(); - hideBrowserSearch(); // Don't do anything if the user entered an empty URL. if (TextUtils.isEmpty(url)) { @@ -1526,13 +1519,13 @@ abstract public class BrowserApp extends GeckoApp return false; } - mBrowserToolbar.cancelEdit(); - - // Resetting the visibility of HomePager, which might have been hidden - // by the filterEditingMode(). + // cancelEdit will call hideHomePager. If we're on web content, this is fine. If we're on + // about:home, the HomePager needs to be visible in the end (note that hideHomePager will + // not hide the HomePager on about:home). However, filterEditingMode may have hidden the + // HomePager so we set it visible here. mHomePager.setVisibility(View.VISIBLE); - hideHomePager(); - hideBrowserSearch(); + + mBrowserToolbar.cancelEdit(); return true; } From c09c677beb53f849b3a82a62d81cf232ce639c9a Mon Sep 17 00:00:00 2001 From: Michael Comella Date: Mon, 14 Oct 2013 17:10:27 -0700 Subject: [PATCH 09/24] Bug 915918 - Part 2: openUrl -> openUrlAndExitEditingMode. r=lucasr --- mobile/android/base/BrowserApp.java | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/mobile/android/base/BrowserApp.java b/mobile/android/base/BrowserApp.java index 1c1bb03c0185..23578fecb666 100644 --- a/mobile/android/base/BrowserApp.java +++ b/mobile/android/base/BrowserApp.java @@ -1324,19 +1324,19 @@ abstract public class BrowserApp extends GeckoApp return true; } - private void openUrl(String url) { - openUrl(url, null, false); + private void openUrlAndStopEditing(String url) { + openUrlAndStopEditing(url, null, false); } - private void openUrl(String url, boolean newTab) { - openUrl(url, null, newTab); + private void openUrlAndStopEditing(String url, boolean newTab) { + openUrlAndStopEditing(url, null, newTab); } - private void openUrl(String url, String searchEngine) { - openUrl(url, searchEngine, false); + private void openUrlAndStopEditing(String url, String searchEngine) { + openUrlAndStopEditing(url, searchEngine, false); } - private void openUrl(String url, String searchEngine, boolean newTab) { + private void openUrlAndStopEditing(String url, String searchEngine, boolean newTab) { mBrowserToolbar.setProgressVisibility(true); int flags = Tabs.LOADURL_NONE; @@ -2356,7 +2356,7 @@ abstract public class BrowserApp extends GeckoApp for (String url : urls) { if (!maybeSwitchToTab(url, flags)) { - openUrl(url, true); + openUrlAndStopEditing(url, true); } } } @@ -2365,7 +2365,7 @@ abstract public class BrowserApp extends GeckoApp @Override public void onUrlOpen(String url, EnumSet flags) { if (!maybeSwitchToTab(url, flags)) { - openUrl(url); + openUrlAndStopEditing(url); } } @@ -2373,7 +2373,7 @@ abstract public class BrowserApp extends GeckoApp @Override public void onSearch(String engineId, String text) { recordSearch(engineId, "barsuggest"); - openUrl(text, engineId); + openUrlAndStopEditing(text, engineId); } // BrowserSearch.OnEditSuggestionListener From 80262df43ef18ca6fee4dadbb529edcc6c1914ab Mon Sep 17 00:00:00 2001 From: Richard Newman Date: Fri, 18 Oct 2013 12:27:57 -0700 Subject: [PATCH 10/24] Bug 925521 - Part 1: remove filter on recorded search engine identifiers. r=gps --- services/healthreport/providers.jsm | 236 ++++++------------ .../tests/xpcshell/test_provider_searches.js | 38 ++- 2 files changed, 106 insertions(+), 168 deletions(-) diff --git a/services/healthreport/providers.jsm b/services/healthreport/providers.jsm index 77e6f7a77df7..0b8075e01569 100644 --- a/services/healthreport/providers.jsm +++ b/services/healthreport/providers.jsm @@ -1186,71 +1186,20 @@ SearchCountMeasurement1.prototype = Object.freeze({ * We don't use the search engine name directly, because it is shared across * locales; e.g., eBay-de and eBay both share the name "eBay". */ -function SearchCountMeasurement2() { - this._fieldSpecs = null; - this._interestingEngines = null; // Name -> ID. ("Amazon.com" -> "amazondotcom") - +function SearchCountMeasurementBase() { + this._fieldSpecs = {}; Metrics.Measurement.call(this); } -SearchCountMeasurement2.prototype = Object.freeze({ +SearchCountMeasurementBase.prototype = Object.freeze({ __proto__: Metrics.Measurement.prototype, - name: "counts", - version: 2, - /** - * Default implementation; can be overridden by test helpers. - */ - getDefaultEngines: function () { - return Services.search.getDefaultEngines(); - }, - - _initialize: function () { - // Don't create all of these for every profile. - // There are 61 partner engines, translating to 244 fields. - // Instead, compute only those that are possible -- those for whom the - // provider is one of the default search engines. - // This set can grow over time, and change as users run different localized - // Firefox instances. - this._fieldSpecs = {}; - this._interestingEngines = {}; - - for (let source of this.SOURCES) { - this._fieldSpecs["other." + source] = DAILY_COUNTER_FIELD; - } - - let engines = this.getDefaultEngines(); - for (let engine of engines) { - let id = engine.identifier; - if (!id || (this.PROVIDERS.indexOf(id) == -1)) { - continue; - } - - this._interestingEngines[engine.name] = id; - let fieldPrefix = id + "."; - for (let source of this.SOURCES) { - this._fieldSpecs[fieldPrefix + source] = DAILY_COUNTER_FIELD; - } - } - }, - - // Our fields are dynamic, so we compute them into _fieldSpecs by looking at - // the current set of interesting engines. + // Our fields are dynamic. get fields() { - if (!this._fieldSpecs) { - this._initialize(); - } return this._fieldSpecs; }, - get interestingEngines() { - if (!this._fieldSpecs) { - this._initialize(); - } - return this._interestingEngines; - }, - /** * Override the default behavior: serializers should include every counter * field from the DB, even if we don't currently have it registered. @@ -1280,101 +1229,6 @@ SearchCountMeasurement2.prototype = Object.freeze({ return Metrics.Storage.FIELD_DAILY_COUNTER; }, - // You can compute the total list of fields by unifying the entire l10n repo - // set with the list of partners: - // - // sort -u */*/searchplugins/list.txt | tr -d '^M' | uniq | grep -f partners.txt - // - // where partners.txt contains - // - // amazon - // aol - // bing - // eBay - // google - // mailru - // mercadolibre - // seznam - // twitter - // yahoo - // yandex - // - // Please update this list as the set of partners changes. - // - PROVIDERS: [ - "amazon-co-uk", - "amazon-de", - "amazon-en-GB", - "amazon-france", - "amazon-it", - "amazon-jp", - "amazondotcn", - "amazondotcom", - "amazondotcom-de", - - "aol-en-GB", - "aol-web-search", - - "bing", - - "eBay", - "eBay-de", - "eBay-en-GB", - "eBay-es", - "eBay-fi", - "eBay-france", - "eBay-hu", - "eBay-in", - "eBay-it", - - "google", - "google-jp", - "google-ku", - "google-maps-zh-TW", - - "mailru", - - "mercadolibre-ar", - "mercadolibre-cl", - "mercadolibre-mx", - - "seznam-cz", - - "twitter", - "twitter-de", - "twitter-ja", - - "yahoo", - "yahoo-NO", - "yahoo-answer-zh-TW", - "yahoo-ar", - "yahoo-bid-zh-TW", - "yahoo-br", - "yahoo-ch", - "yahoo-cl", - "yahoo-de", - "yahoo-en-GB", - "yahoo-es", - "yahoo-fi", - "yahoo-france", - "yahoo-fy-NL", - "yahoo-id", - "yahoo-in", - "yahoo-it", - "yahoo-jp", - "yahoo-jp-auctions", - "yahoo-mx", - "yahoo-sv-SE", - "yahoo-zh-TW", - - "yandex", - "yandex-ru", - "yandex-slovari", - "yandex-tr", - "yandex.by", - "yandex.ru-be", - ], - SOURCES: [ "abouthome", "contextmenu", @@ -1383,6 +1237,59 @@ SearchCountMeasurement2.prototype = Object.freeze({ ], }); +function SearchCountMeasurement2() { + SearchCountMeasurementBase.call(this); +} + +SearchCountMeasurement2.prototype = Object.freeze({ + __proto__: SearchCountMeasurementBase.prototype, + name: "counts", + version: 2, +}); + +function SearchCountMeasurement3() { + this.nameMappings = null; + SearchCountMeasurementBase.call(this); +} + +SearchCountMeasurement3.prototype = Object.freeze({ + __proto__: SearchCountMeasurementBase.prototype, + name: "counts", + version: 3, + + getEngines: function () { + return Services.search.getEngines(); + }, + + _initialize: function () { + this.nameMappings = {}; + let engines = this.getEngines(); + for (let engine of engines) { + let name = engine.name; + if (!name) { + // This is something we'd like to know, but we can't track it unless we + // rejig how recordSearchInHealthReport is implemented. + continue; + } + + let id = engine.identifier; + if (!id) { + continue; + } + + // TODO: again, we need to rejig this to avoid name collisions. + this.nameMappings[name] = id; + } + }, + + getEngineID: function (engineName) { + if (!this.nameMappings) { + this._initialize(); + } + return this.nameMappings[engineName] || "other-" + engineName; + }, +}); + this.SearchesProvider = function () { Metrics.Provider.call(this); }; @@ -1394,6 +1301,7 @@ this.SearchesProvider.prototype = Object.freeze({ measurementTypes: [ SearchCountMeasurement1, SearchCountMeasurement2, + SearchCountMeasurement3, ], /** @@ -1413,7 +1321,8 @@ this.SearchesProvider.prototype = Object.freeze({ * * @param engine * (string) The search engine used. If the search engine is unknown, - * the search will be attributed to "other". + * the search will be attributed to "other-$engine"; otherwise, its + * identifier will be used. * @param source * (string) Where the search was initiated from. Must be one of the * SearchCountMeasurement2.SOURCES values. @@ -1422,17 +1331,30 @@ this.SearchesProvider.prototype = Object.freeze({ * The promise is resolved when the storage operation completes. */ recordSearch: function (engine, source) { - let m = this.getMeasurement("counts", 2); + let m = this.getMeasurement("counts", 3); if (m.SOURCES.indexOf(source) == -1) { throw new Error("Unknown source for search: " + source); } - let id = m.interestingEngines[engine] || "other"; - let field = id + "." + source; - return this.enqueueStorageOperation(function recordSearch() { - return m.incrementDailyCounter(field); - }); + let field = m.getEngineID(engine) + "." + source; + if (this.storage.hasFieldFromMeasurement(m.id, field, + this.storage.FIELD_DAILY_COUNTER)) { + let fieldID = this.storage.fieldIDFromMeasurement(m.id, field); + return this.enqueueStorageOperation(function recordSearchKnownField() { + return this.storage.incrementDailyCounterFromFieldID(fieldID); + }.bind(this)); + } + + // Otherwise, we first need to create the field. + return this.enqueueStorageOperation(function recordFieldAndSearch() { + // This function has to return a promise. + return Task.spawn(function () { + let fieldID = yield this.storage.registerField(m.id, field, + this.storage.FIELD_DAILY_COUNTER); + yield this.storage.incrementDailyCounterFromFieldID(fieldID); + }.bind(this)); + }.bind(this)); }, }); diff --git a/services/healthreport/tests/xpcshell/test_provider_searches.js b/services/healthreport/tests/xpcshell/test_provider_searches.js index fbeffa5991d9..e90a2bb7e44e 100644 --- a/services/healthreport/tests/xpcshell/test_provider_searches.js +++ b/services/healthreport/tests/xpcshell/test_provider_searches.js @@ -17,11 +17,12 @@ const DEFAULT_ENGINES = [ ]; function MockSearchCountMeasurement() { - bsp.SearchCountMeasurement2.call(this); + bsp.SearchCountMeasurement3.call(this); } MockSearchCountMeasurement.prototype = { - __proto__: bsp.SearchCountMeasurement2.prototype, - getDefaultEngines: function () { + __proto__: bsp.SearchCountMeasurement3.prototype, + + getEngines: function () { return DEFAULT_ENGINES; }, }; @@ -52,7 +53,12 @@ add_task(function test_record() { let now = new Date(); - for (let engine of DEFAULT_ENGINES) { + // Record searches for all but one of our defaults, and one engine that's + // not a default. + for (let engine of DEFAULT_ENGINES.concat([{name: "Not Default", identifier: "notdef"}])) { + if (engine.identifier == "yahoo") { + continue; + } yield provider.recordSearch(engine.name, "abouthome"); yield provider.recordSearch(engine.name, "contextmenu"); yield provider.recordSearch(engine.name, "searchbar"); @@ -69,7 +75,7 @@ add_task(function test_record() { do_check_true(errored); } - let m = provider.getMeasurement("counts", 2); + let m = provider.getMeasurement("counts", 3); let data = yield m.getValues(); do_check_eq(data.days.size, 1); do_check_true(data.days.hasDay(now)); @@ -77,17 +83,27 @@ add_task(function test_record() { let day = data.days.getDay(now); for (let engine of DEFAULT_ENGINES) { let identifier = engine.identifier; - if (identifier == "foobar") { - identifier = "other"; - } + let expected = identifier != "yahoo"; for (let source of ["abouthome", "contextmenu", "searchbar", "urlbar"]) { let field = identifier + "." + source; - do_check_true(day.has(field)); - do_check_eq(day.get(field), 1); + if (expected) { + do_check_true(day.has(field)); + do_check_eq(day.get(field), 1); + } else { + do_check_false(day.has(field)); + } } } + // Also, check that our non-default engine contributed, with a computed + // identifier. + let identifier = "other-Not Default"; + for (let source of ["abouthome", "contextmenu", "searchbar", "urlbar"]) { + let field = identifier + "." + source; + do_check_true(day.has(field)); + } + yield storage.close(); }); @@ -96,7 +112,7 @@ add_task(function test_includes_other_fields() { let provider = new MockSearchesProvider(); yield provider.init(storage); - let m = provider.getMeasurement("counts", 2); + let m = provider.getMeasurement("counts", 3); // Register a search against a provider that isn't live in this session. let id = yield m.storage.registerField(m.id, "test.searchbar", From 286bec197420f72e416b81fb1fe8a530bab6dd24 Mon Sep 17 00:00:00 2001 From: Richard Newman Date: Fri, 18 Oct 2013 12:31:39 -0700 Subject: [PATCH 11/24] Bug 925521 - Part 2: correctly record identifiers for non-pre-installed engines. r=gps * * * Bug 925521 - Review comments. --- browser/base/content/browser.js | 3 +- browser/components/nsBrowserGlue.js | 5 ++- browser/components/search/content/search.xml | 2 +- browser/modules/AboutHome.jsm | 5 +-- services/healthreport/providers.jsm | 35 ++++--------------- .../tests/xpcshell/test_provider_searches.js | 16 ++++----- 6 files changed, 20 insertions(+), 46 deletions(-) diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js index 2e760b5a998a..5f066f690268 100644 --- a/browser/base/content/browser.js +++ b/browser/base/content/browser.js @@ -3083,8 +3083,7 @@ const BrowserSearch = { * FHR records only search counts and nothing pertaining to the search itself. * * @param engine - * (string) The name of the engine used to perform the search. This - * is typically nsISearchEngine.name. + * (nsISearchEngine) The engine handling the search. * @param source * (string) Where the search originated from. See the FHR * SearchesProvider for allowed values. diff --git a/browser/components/nsBrowserGlue.js b/browser/components/nsBrowserGlue.js index efb2e60525c5..ff030f5ce048 100644 --- a/browser/components/nsBrowserGlue.js +++ b/browser/components/nsBrowserGlue.js @@ -318,9 +318,8 @@ BrowserGlue.prototype = { reporter.onInit().then(function record() { try { - let name = subject.QueryInterface(Ci.nsISearchEngine).name; - reporter.getProvider("org.mozilla.searches").recordSearch(name, - "urlbar"); + let engine = subject.QueryInterface(Ci.nsISearchEngine); + reporter.getProvider("org.mozilla.searches").recordSearch(engine, "urlbar"); } catch (ex) { Cu.reportError(ex); } diff --git a/browser/components/search/content/search.xml b/browser/components/search/content/search.xml index ce3cdc15f171..58fd87e7d146 100644 --- a/browser/components/search/content/search.xml +++ b/browser/components/search/content/search.xml @@ -483,7 +483,7 @@ // null parameter below specifies HTML response for search var submission = this.currentEngine.getSubmission(aData, null, "searchbar"); - BrowserSearch.recordSearchInHealthReport(this.currentEngine.name, "searchbar"); + BrowserSearch.recordSearchInHealthReport(this.currentEngine, "searchbar"); openUILinkIn(submission.uri.spec, aWhere, null, submission.postData); ]]> diff --git a/browser/modules/AboutHome.jsm b/browser/modules/AboutHome.jsm index 98c443d11cdd..4d357476d2d8 100644 --- a/browser/modules/AboutHome.jsm +++ b/browser/modules/AboutHome.jsm @@ -165,11 +165,12 @@ let AboutHome = { Cu.reportError(ex); break; } + let engine = Services.search.currentEngine; #ifdef MOZ_SERVICES_HEALTHREPORT - window.BrowserSearch.recordSearchInHealthReport(data.engineName, "abouthome"); + window.BrowserSearch.recordSearchInHealthReport(engine, "abouthome"); #endif // Trigger a search through nsISearchEngine.getSubmission() - let submission = Services.search.currentEngine.getSubmission(data.searchTerms, null, "homepage"); + let submission = engine.getSubmission(data.searchTerms, null, "homepage"); window.loadURI(submission.uri.spec, null, submission.postData); break; } diff --git a/services/healthreport/providers.jsm b/services/healthreport/providers.jsm index 0b8075e01569..fdb6ae06f815 100644 --- a/services/healthreport/providers.jsm +++ b/services/healthreport/providers.jsm @@ -1248,7 +1248,6 @@ SearchCountMeasurement2.prototype = Object.freeze({ }); function SearchCountMeasurement3() { - this.nameMappings = null; SearchCountMeasurementBase.call(this); } @@ -1261,32 +1260,14 @@ SearchCountMeasurement3.prototype = Object.freeze({ return Services.search.getEngines(); }, - _initialize: function () { - this.nameMappings = {}; - let engines = this.getEngines(); - for (let engine of engines) { - let name = engine.name; - if (!name) { - // This is something we'd like to know, but we can't track it unless we - // rejig how recordSearchInHealthReport is implemented. - continue; - } - - let id = engine.identifier; - if (!id) { - continue; - } - - // TODO: again, we need to rejig this to avoid name collisions. - this.nameMappings[name] = id; + getEngineID: function (engine) { + if (!engine) { + return "other"; } - }, - - getEngineID: function (engineName) { - if (!this.nameMappings) { - this._initialize(); + if (engine.identifier) { + return engine.identifier; } - return this.nameMappings[engineName] || "other-" + engineName; + return "other-" + engine.name; }, }); @@ -1320,9 +1301,7 @@ this.SearchesProvider.prototype = Object.freeze({ * Record that a search occurred. * * @param engine - * (string) The search engine used. If the search engine is unknown, - * the search will be attributed to "other-$engine"; otherwise, its - * identifier will be used. + * (nsISearchEngine) The search engine used. * @param source * (string) Where the search was initiated from. Must be one of the * SearchCountMeasurement2.SOURCES values. diff --git a/services/healthreport/tests/xpcshell/test_provider_searches.js b/services/healthreport/tests/xpcshell/test_provider_searches.js index e90a2bb7e44e..94ce79760857 100644 --- a/services/healthreport/tests/xpcshell/test_provider_searches.js +++ b/services/healthreport/tests/xpcshell/test_provider_searches.js @@ -21,10 +21,6 @@ function MockSearchCountMeasurement() { } MockSearchCountMeasurement.prototype = { __proto__: bsp.SearchCountMeasurement3.prototype, - - getEngines: function () { - return DEFAULT_ENGINES; - }, }; function MockSearchesProvider() { @@ -59,16 +55,16 @@ add_task(function test_record() { if (engine.identifier == "yahoo") { continue; } - yield provider.recordSearch(engine.name, "abouthome"); - yield provider.recordSearch(engine.name, "contextmenu"); - yield provider.recordSearch(engine.name, "searchbar"); - yield provider.recordSearch(engine.name, "urlbar"); + yield provider.recordSearch(engine, "abouthome"); + yield provider.recordSearch(engine, "contextmenu"); + yield provider.recordSearch(engine, "searchbar"); + yield provider.recordSearch(engine, "urlbar"); } // Invalid sources should throw. let errored = false; try { - yield provider.recordSearch(DEFAULT_ENGINES[0].name, "bad source"); + yield provider.recordSearch(DEFAULT_ENGINES[0], "bad source"); } catch (ex) { errored = true; } finally { @@ -98,7 +94,7 @@ add_task(function test_record() { // Also, check that our non-default engine contributed, with a computed // identifier. - let identifier = "other-Not Default"; + let identifier = "notdef"; for (let source of ["abouthome", "contextmenu", "searchbar", "urlbar"]) { let field = identifier + "." + source; do_check_true(day.has(field)); From 9472410b0e265a90657dd1c0421f4a9588e89d47 Mon Sep 17 00:00:00 2001 From: Richard Newman Date: Fri, 18 Oct 2013 17:43:42 -0700 Subject: [PATCH 12/24] Bug 925521 - Part 3: follow-up to fix browser tests. r=trivial --- browser/base/content/test/general/browser_aboutHome.js | 2 +- browser/components/search/test/browser_healthreport.js | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/browser/base/content/test/general/browser_aboutHome.js b/browser/base/content/test/general/browser_aboutHome.js index 85cd863282f4..10a268bd153b 100644 --- a/browser/base/content/test/general/browser_aboutHome.js +++ b/browser/base/content/test/general/browser_aboutHome.js @@ -486,7 +486,7 @@ function getNumberOfSearches(aEngineName) { let provider = reporter.getProvider("org.mozilla.searches"); ok(provider, "Searches provider is available."); - let m = provider.getMeasurement("counts", 2); + let m = provider.getMeasurement("counts", 3); return m.getValues().then(data => { let now = new Date(); let yday = new Date(now); diff --git a/browser/components/search/test/browser_healthreport.js b/browser/components/search/test/browser_healthreport.js index 0cbe2e88c214..58e55ba21ecc 100644 --- a/browser/components/search/test/browser_healthreport.js +++ b/browser/components/search/test/browser_healthreport.js @@ -27,14 +27,15 @@ function test() { ok(reporter, "Health Reporter available."); reporter.onInit().then(function onInit() { let provider = reporter.getProvider("org.mozilla.searches"); - let m = provider.getMeasurement("counts", 2); + let m = provider.getMeasurement("counts", 3); m.getValues().then(function onData(data) { let now = new Date(); let oldCount = 0; - // Foo engine goes into "other" bucket. - let field = "other.searchbar"; + // Find the right bucket for the "Foo" engine. + let engine = Services.search.getEngineByName("Foo"); + let field = (engine.identifier || "other-Foo") + ".searchbar"; if (data.days.hasDay(now)) { let day = data.days.getDay(now); From 25f479a6ac34ad91476aca2c66ca2c1f0d352ddd Mon Sep 17 00:00:00 2001 From: Richard Newman Date: Fri, 18 Oct 2013 13:13:37 -0700 Subject: [PATCH 13/24] Bug 925517 - Remove filter on recorded search engine identifiers for Fennec. r=mcomella --- mobile/android/base/BrowserApp.java | 18 +-- .../base/health/BrowserHealthRecorder.java | 103 +----------------- mobile/android/base/home/BrowserSearch.java | 23 ++-- mobile/android/base/home/SearchEngine.java | 88 +++++++++++++-- mobile/android/base/home/SearchEngineRow.java | 35 +++--- 5 files changed, 119 insertions(+), 148 deletions(-) diff --git a/mobile/android/base/BrowserApp.java b/mobile/android/base/BrowserApp.java index cb9183e68bfd..899f8abb652f 100644 --- a/mobile/android/base/BrowserApp.java +++ b/mobile/android/base/BrowserApp.java @@ -19,6 +19,7 @@ import org.mozilla.gecko.health.BrowserHealthReporter; import org.mozilla.gecko.home.BrowserSearch; import org.mozilla.gecko.home.HomePager; import org.mozilla.gecko.home.HomePager.OnUrlOpenListener; +import org.mozilla.gecko.home.SearchEngine; import org.mozilla.gecko.menu.GeckoMenu; import org.mozilla.gecko.prompts.Prompt; import org.mozilla.gecko.util.Clipboard; @@ -1485,15 +1486,18 @@ abstract public class BrowserApp extends GeckoApp /** * Record in Health Report that a search has occurred. * - * @param identifier - * a search identifier, such as "partnername". Can be null. + * @param engine + * a search engine instance. Can be null. * @param where * where the search was initialized; one of the values in * {@link BrowserHealthRecorder#SEARCH_LOCATIONS}. */ - private static void recordSearch(String identifier, String where) { - Log.i(LOGTAG, "Recording search: " + identifier + ", " + where); + private static void recordSearch(SearchEngine engine, String where) { + Log.i(LOGTAG, "Recording search: " + + ((engine == null) ? "null" : engine.name) + + ", " + where); try { + String identifier = (engine == null) ? "other" : engine.getEngineIdentifier(); JSONObject message = new JSONObject(); message.put("type", BrowserHealthRecorder.EVENT_SEARCH); message.put("location", where); @@ -2308,9 +2312,9 @@ abstract public class BrowserApp extends GeckoApp // BrowserSearch.OnSearchListener @Override - public void onSearch(String engineId, String text) { - recordSearch(engineId, "barsuggest"); - openUrl(text, engineId); + public void onSearch(SearchEngine engine, String text) { + recordSearch(engine, "barsuggest"); + openUrl(text, engine.name); } // BrowserSearch.OnEditSuggestionListener diff --git a/mobile/android/base/health/BrowserHealthRecorder.java b/mobile/android/base/health/BrowserHealthRecorder.java index a35699c5b6be..fa4ba4eaeb76 100644 --- a/mobile/android/base/health/BrowserHealthRecorder.java +++ b/mobile/android/base/health/BrowserHealthRecorder.java @@ -741,7 +741,7 @@ public class BrowserHealthRecorder implements GeckoEventListener { */ public static final String MEASUREMENT_NAME_SEARCH_COUNTS = "org.mozilla.searches.counts"; - public static final int MEASUREMENT_VERSION_SEARCH_COUNTS = 4; + public static final int MEASUREMENT_VERSION_SEARCH_COUNTS = 5; public static final String[] SEARCH_LOCATIONS = { "barkeyword", @@ -749,84 +749,6 @@ public class BrowserHealthRecorder implements GeckoEventListener { "bartext", }; - // See services/healthreport/providers.jsm. Sorry for the duplication. - // THIS LIST MUST BE SORTED per java.lang.Comparable. - private static final String[] SEARCH_PROVIDERS = { - "amazon-co-uk", - "amazon-de", - "amazon-en-GB", - "amazon-france", - "amazon-it", - "amazon-jp", - "amazondotcn", - "amazondotcom", - "amazondotcom-de", - - "aol-en-GB", - "aol-web-search", - - "bing", - - "eBay", - "eBay-de", - "eBay-en-GB", - "eBay-es", - "eBay-fi", - "eBay-france", - "eBay-hu", - "eBay-in", - "eBay-it", - - "google", - "google-jp", - "google-ku", - "google-maps-zh-TW", - - "mailru", - - "mercadolibre-ar", - "mercadolibre-cl", - "mercadolibre-mx", - - "seznam-cz", - - "twitter", - "twitter-de", - "twitter-ja", - - "wikipedia", // Manually added. - - "yahoo", - "yahoo-NO", - "yahoo-answer-zh-TW", - "yahoo-ar", - "yahoo-bid-zh-TW", - "yahoo-br", - "yahoo-ch", - "yahoo-cl", - "yahoo-de", - "yahoo-en-GB", - "yahoo-es", - "yahoo-fi", - "yahoo-france", - "yahoo-fy-NL", - "yahoo-id", - "yahoo-in", - "yahoo-it", - "yahoo-jp", - "yahoo-jp-auctions", - "yahoo-mx", - "yahoo-sv-SE", - "yahoo-zh-TW", - - "yandex", - "yandex-ru", - "yandex-slovari", - "yandex-tr", - "yandex.by", - "yandex.ru-be", - }; - private void initializeSearchProvider() { this.storage.ensureMeasurementInitialized( MEASUREMENT_NAME_SEARCH_COUNTS, @@ -854,30 +776,13 @@ public class BrowserHealthRecorder implements GeckoEventListener { this.dispatcher.registerEventListener(EVENT_SEARCH, this); } - /** - * Return the field key for the search provider. This turns null and - * non-partner providers into "other". - * - * @param engine an engine identifier, such as "yandex" - * @return the key to use, such as "other" or "yandex". - */ - protected String getEngineKey(final String engine) { - if (engine == null) { - return "other"; - } - - // This is inefficient. Optimize if necessary. - boolean found = (0 <= java.util.Arrays.binarySearch(SEARCH_PROVIDERS, engine)); - return found ? engine : "other"; - } - /** * Record a search. * - * @param engine the string identifier for the engine, or null if it's not a partner. + * @param engineID the string identifier for the engine. Can be null. * @param location one of a fixed set of locations: see {@link #SEARCH_LOCATIONS}. */ - public void recordSearch(final String engine, final String location) { + public void recordSearch(final String engineID, final String location) { if (this.state != State.INITIALIZED) { Log.d(LOG_TAG, "Not initialized: not recording search. (" + this.state + ")"); return; @@ -889,7 +794,7 @@ public class BrowserHealthRecorder implements GeckoEventListener { final int day = storage.getDay(); final int env = this.env; - final String key = getEngineKey(engine); + final String key = (engineID == null) ? "other" : engineID; final BrowserHealthRecorder self = this; ThreadUtils.postToBackgroundThread(new Runnable() { diff --git a/mobile/android/base/home/BrowserSearch.java b/mobile/android/base/home/BrowserSearch.java index 2ca0df7a7eea..d0e52d6f95f3 100644 --- a/mobile/android/base/home/BrowserSearch.java +++ b/mobile/android/base/home/BrowserSearch.java @@ -13,8 +13,8 @@ import org.mozilla.gecko.R; import org.mozilla.gecko.Tab; import org.mozilla.gecko.Tabs; import org.mozilla.gecko.db.BrowserDB.URLColumns; -import org.mozilla.gecko.gfx.BitmapUtils; import org.mozilla.gecko.home.HomePager.OnUrlOpenListener; +import org.mozilla.gecko.home.SearchEngine; import org.mozilla.gecko.home.SearchLoader.SearchCursorLoader; import org.mozilla.gecko.util.GeckoEventListener; import org.mozilla.gecko.util.StringUtils; @@ -132,7 +132,7 @@ public class BrowserSearch extends HomeFragment private View mSuggestionsOptInPrompt; public interface OnSearchListener { - public void onSearch(String engineId, String text); + public void onSearch(SearchEngine engine, String text); } public interface OnEditSuggestionListener { @@ -393,7 +393,7 @@ public class BrowserSearch extends HomeFragment } private void setSuggestions(ArrayList suggestions) { - mSearchEngines.get(0).suggestions = suggestions; + mSearchEngines.get(0).setSuggestions(suggestions); mAdapter.notifyDataSetChanged(); } @@ -417,14 +417,11 @@ public class BrowserSearch extends HomeFragment ArrayList searchEngines = new ArrayList(); for (int i = 0; i < engines.length(); i++) { final JSONObject engineJSON = engines.getJSONObject(i); - final String name = engineJSON.getString("name"); - final String identifier = engineJSON.getString("identifier"); - final String iconURI = engineJSON.getString("iconURI"); - final Bitmap icon = BitmapUtils.getBitmapFromDataURI(iconURI); + final SearchEngine engine = new SearchEngine(engineJSON); - if (name.equals(suggestEngine) && suggestTemplate != null) { - // Suggest engine should be at the front of the list - searchEngines.add(0, new SearchEngine(name, identifier, icon)); + if (engine.name.equals(suggestEngine) && suggestTemplate != null) { + // Suggest engine should be at the front of the list. + searchEngines.add(0, engine); // The only time Tabs.getInstance().getSelectedTab() should // be null is when we're restoring after a crash. We should @@ -441,7 +438,7 @@ public class BrowserSearch extends HomeFragment SUGGESTION_TIMEOUT, SUGGESTION_MAX); } } else { - searchEngines.add(new SearchEngine(name, identifier, icon)); + searchEngines.add(engine); } } @@ -713,7 +710,7 @@ public class BrowserSearch extends HomeFragment // row contains multiple items, clicking the row will do nothing. final int index = getEngineIndex(position); if (index != -1) { - return mSearchEngines.get(index).suggestions.isEmpty(); + return !mSearchEngines.get(index).hasSuggestions(); } return true; @@ -744,7 +741,7 @@ public class BrowserSearch extends HomeFragment row.setSearchTerm(mSearchTerm); final SearchEngine engine = mSearchEngines.get(getEngineIndex(position)); - final boolean animate = (mAnimateSuggestions && engine.suggestions.size() > 0); + final boolean animate = (mAnimateSuggestions && engine.hasSuggestions()); row.updateFromSearchEngine(engine, animate); if (animate) { // Only animate suggestions the first time they are shown diff --git a/mobile/android/base/home/SearchEngine.java b/mobile/android/base/home/SearchEngine.java index b82f7ab7befb..0ee241f5646e 100644 --- a/mobile/android/base/home/SearchEngine.java +++ b/mobile/android/base/home/SearchEngine.java @@ -5,25 +5,89 @@ package org.mozilla.gecko.home; +import org.mozilla.gecko.gfx.BitmapUtils; + +import org.json.JSONException; +import org.json.JSONObject; + import android.graphics.Bitmap; +import android.util.Log; import java.util.ArrayList; +import java.util.List; -class SearchEngine { - public String name; - public String identifier; - public Bitmap icon; - public ArrayList suggestions; +public class SearchEngine { + public static final String LOG_TAG = "GeckoSearchEngine"; - public SearchEngine(String name, String identifier) { - this(name, identifier, null); + public final String name; // Never null. + public final String identifier; // Can be null. + + private final Bitmap icon; + private volatile List suggestions = new ArrayList(); // Never null. + + public SearchEngine(JSONObject engineJSON) throws JSONException { + if (engineJSON == null) { + throw new IllegalArgumentException("Can't instantiate SearchEngine from null JSON."); + } + + this.name = getString(engineJSON, "name"); + if (this.name == null) { + throw new IllegalArgumentException("Cannot have an unnamed search engine."); + } + + this.identifier = getString(engineJSON, "identifier"); + + final String iconURI = getString(engineJSON, "iconURI"); + if (iconURI == null) { + Log.w(LOG_TAG, "iconURI is null for search engine " + this.name); + this.icon = null; + return; + } + this.icon = BitmapUtils.getBitmapFromDataURI(iconURI); } - public SearchEngine(String name, String identifier, Bitmap icon) { - this.name = name; - this.identifier = identifier; - this.icon = icon; - this.suggestions = new ArrayList(); + private static String getString(JSONObject data, String key) throws JSONException { + if (data.isNull(key)) { + return null; + } + return data.getString(key); + } + + /** + * @return a non-null string suitable for use by FHR. + */ + public String getEngineIdentifier() { + if (this.identifier != null) { + return this.identifier; + } + if (this.name != null) { + return "other-" + this.name; + } + return "other"; + } + + public boolean hasSuggestions() { + return !this.suggestions.isEmpty(); + } + + public int getSuggestionsCount() { + return this.suggestions.size(); + } + + public Iterable getSuggestions() { + return this.suggestions; + } + + public void setSuggestions(List suggestions) { + if (suggestions == null) { + this.suggestions = new ArrayList(); + return; + } + this.suggestions = suggestions; + } + + public Bitmap getIcon() { + return this.icon; } } diff --git a/mobile/android/base/home/SearchEngineRow.java b/mobile/android/base/home/SearchEngineRow.java index ea60486f42f3..a317b73b2162 100644 --- a/mobile/android/base/home/SearchEngineRow.java +++ b/mobile/android/base/home/SearchEngineRow.java @@ -82,7 +82,7 @@ class SearchEngineRow extends AnimatedHeightLayout { mUrlOpenListener.onUrlOpen(suggestion, EnumSet.noneOf(OnUrlOpenListener.Flags.class)); } } else if (mSearchListener != null) { - mSearchListener.onSearch(mSearchEngine.name, suggestion); + mSearchListener.onSearch(mSearchEngine, suggestion); } } }; @@ -135,7 +135,7 @@ class SearchEngineRow extends AnimatedHeightLayout { public void performUserEnteredSearch() { String searchTerm = getSuggestionTextFromView(mUserEnteredView); if (mSearchListener != null) { - mSearchListener.onSearch(mSearchEngine.name, searchTerm); + mSearchListener.onSearch(mSearchEngine, searchTerm); } } @@ -162,25 +162,25 @@ class SearchEngineRow extends AnimatedHeightLayout { } public void updateFromSearchEngine(SearchEngine searchEngine, boolean animate) { - // Update search engine reference + // Update search engine reference. mSearchEngine = searchEngine; - // Set the search engine icon (e.g., Google) for the row - mIconView.updateAndScaleImage(mSearchEngine.icon, mSearchEngine.name); + // Set the search engine icon (e.g., Google) for the row. + mIconView.updateAndScaleImage(mSearchEngine.getIcon(), mSearchEngine.getEngineIdentifier()); - // Set the initial content description + // Set the initial content description. setDescriptionOnSuggestion(mUserEnteredTextView, mUserEnteredTextView.getText().toString()); - // Add additional suggestions given by this engine + // Add additional suggestions given by this engine. final int recycledSuggestionCount = mSuggestionView.getChildCount(); - final int suggestionCount = mSearchEngine.suggestions.size(); - for (int i = 0; i < suggestionCount; i++) { + int suggestionCounter = 0; + for (String suggestion : mSearchEngine.getSuggestions()) { final View suggestionItem; - // Reuse suggestion views from recycled view, if possible - if (i + 1 < recycledSuggestionCount) { - suggestionItem = mSuggestionView.getChildAt(i + 1); + // Reuse suggestion views from recycled view, if possible. + if (suggestionCounter + 1 < recycledSuggestionCount) { + suggestionItem = mSuggestionView.getChildAt(suggestionCounter + 1); suggestionItem.setVisibility(View.VISIBLE); } else { suggestionItem = mInflater.inflate(R.layout.suggestion_item, null); @@ -195,23 +195,24 @@ class SearchEngineRow extends AnimatedHeightLayout { mSuggestionView.addView(suggestionItem); } - final String suggestion = mSearchEngine.suggestions.get(i); setSuggestionOnView(suggestionItem, suggestion); if (animate) { AlphaAnimation anim = new AlphaAnimation(0, 1); anim.setDuration(ANIMATION_DURATION); - anim.setStartOffset(i * ANIMATION_DURATION); + anim.setStartOffset(suggestionCounter * ANIMATION_DURATION); suggestionItem.startAnimation(anim); } + + ++suggestionCounter; } - // Hide extra suggestions that have been recycled - for (int i = suggestionCount + 1; i < recycledSuggestionCount; i++) { + // Hide extra suggestions that have been recycled. + for (int i = suggestionCounter + 1; i < recycledSuggestionCount; ++i) { mSuggestionView.getChildAt(i).setVisibility(View.GONE); } - // Make sure mSelectedView is still valid + // Make sure mSelectedView is still valid. if (mSelectedView >= mSuggestionView.getChildCount()) { mSelectedView = mSuggestionView.getChildCount() - 1; } From d855131238ea93e384b99fbc74c98b7efdccb167 Mon Sep 17 00:00:00 2001 From: Richard Newman Date: Mon, 28 Oct 2013 21:37:35 -0700 Subject: [PATCH 14/24] Bug 925521 - Follow-up: change measurement version in browser_contextSearchTabPosition.js. r=me --- .../content/test/general/browser_contextSearchTabPosition.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/browser/base/content/test/general/browser_contextSearchTabPosition.js b/browser/base/content/test/general/browser_contextSearchTabPosition.js index 38dbc2adf18b..e05554f71984 100644 --- a/browser/base/content/test/general/browser_contextSearchTabPosition.js +++ b/browser/base/content/test/general/browser_contextSearchTabPosition.js @@ -47,7 +47,7 @@ function test() { let provider = reporter.getProvider("org.mozilla.searches"); ok(provider, "Searches provider is available."); - let m = provider.getMeasurement("counts", 2); + let m = provider.getMeasurement("counts", 3); m.getValues().then(function onValues(data) { let now = new Date(); ok(data.days.hasDay(now), "Have data for today."); From cbfcd6d016cdda4d0ef35d36a759bdea2b7f29ca Mon Sep 17 00:00:00 2001 From: Richard Newman Date: Tue, 29 Oct 2013 09:24:46 -0700 Subject: [PATCH 15/24] Bug 925521 - Follow-up: context search changes. r=gps --- browser/base/content/browser.js | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js index d2e2d3df30da..d129269f3e86 100644 --- a/browser/base/content/browser.js +++ b/browser/base/content/browser.js @@ -3015,11 +3015,11 @@ const BrowserSearch = { * allows the search service to provide a different nsISearchSubmission * depending on e.g. where the search is triggered in the UI. * - * @return string Name of the search engine used to perform a search or null - * if a search was not performed. + * @return engine The search engine used to perform a search, or null if no + * search was performed. */ - loadSearch: function BrowserSearch_search(searchText, useNewTab, purpose) { - var engine; + _loadSearch: function (searchText, useNewTab, purpose) { + let engine; // If the search bar is visible, use the current engine, otherwise, fall // back to the default engine. @@ -3028,7 +3028,7 @@ const BrowserSearch = { else engine = Services.search.defaultEngine; - var submission = engine.getSubmission(searchText, null, purpose); // HTML response + let submission = engine.getSubmission(searchText, null, purpose); // HTML response // getSubmission can return null if the engine doesn't have a URL // with a text/html response type. This is unlikely (since @@ -3045,6 +3045,20 @@ const BrowserSearch = { inBackground: inBackground, relatedToCurrent: true }); + return engine; + }, + + /** + * Just like _loadSearch, but preserving an old API. + * + * @return string Name of the search engine used to perform a search or null + * if a search was not performed. + */ + loadSearch: function BrowserSearch_search(searchText, useNewTab, purpose) { + let engine = BrowserSearch._loadSearch(searchText, useNewTab, purpose); + if (!engine) { + return null; + } return engine.name; }, @@ -3055,7 +3069,7 @@ const BrowserSearch = { * BrowserSearch.loadSearch for the preferred API. */ loadSearchFromContext: function (terms) { - let engine = BrowserSearch.loadSearch(terms, true, "contextmenu"); + let engine = BrowserSearch._loadSearch(terms, true, "contextmenu"); if (engine) { BrowserSearch.recordSearchInHealthReport(engine, "contextmenu"); } From 960780453bc35e7802a49f80e678a8307bea8ebb Mon Sep 17 00:00:00 2001 From: Dave Camp Date: Tue, 29 Oct 2013 10:47:16 -0700 Subject: [PATCH 16/24] Bug 897194: Use outer window ID to match toolbox to tab rather than assuming the currently-selected tab. r=bgrins --- browser/devtools/framework/target.js | 15 ++++++++++++++- toolkit/devtools/server/actors/webbrowser.js | 7 ++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/browser/devtools/framework/target.js b/browser/devtools/framework/target.js index ca1a2a75aa45..eb81d5be0cd0 100644 --- a/browser/devtools/framework/target.js +++ b/browser/devtools/framework/target.js @@ -306,7 +306,20 @@ TabTarget.prototype = { this._client.connect((aType, aTraits) => { this._client.listTabs(aResponse => { this._root = aResponse; - this._form = aResponse.tabs[aResponse.selected]; + + let windowUtils = this.window + .QueryInterface(Ci.nsIInterfaceRequestor) + .getInterface(Ci.nsIDOMWindowUtils); + let outerWindow = windowUtils.outerWindowID; + aResponse.tabs.some((tab) => { + if (tab.outerWindowID === outerWindow) { + this._form = tab; + return true; + } + }); + if (!this._form) { + this._form = aResponse.tabs[aResponse.selected]; + } attachTab(); }); }); diff --git a/toolkit/devtools/server/actors/webbrowser.js b/toolkit/devtools/server/actors/webbrowser.js index 761ce3a13ac1..d4be546666a9 100644 --- a/toolkit/devtools/server/actors/webbrowser.js +++ b/toolkit/devtools/server/actors/webbrowser.js @@ -583,10 +583,15 @@ BrowserTabActor.prototype = { dbg_assert(this.actorID, "tab should have an actorID."); + let windowUtils = this.window + .QueryInterface(Ci.nsIInterfaceRequestor) + .getInterface(Ci.nsIDOMWindowUtils); + let response = { actor: this.actorID, title: this.title, - url: this.url + url: this.url, + outerWindowID: windowUtils.outerWindowID }; // Walk over tab actors added by extensions and add them to a new ActorPool. From 3341ed1e3c94054f0bebad4a7b0fb2cadda37180 Mon Sep 17 00:00:00 2001 From: Brian Nicholson Date: Tue, 29 Oct 2013 10:52:31 -0700 Subject: [PATCH 17/24] Bug 929865 - Use to wrap activities in generated namespace. r=nalexander --HG-- rename : mobile/android/base/WebAppImpl.java => mobile/android/base/WebApp.java --- mobile/android/base/AndroidManifest.xml.in | 32 ++++++++++++++++--- mobile/android/base/App.java.in | 15 --------- mobile/android/base/BrowserApp.java | 18 +++++------ .../base/{WebAppImpl.java => WebApp.java} | 4 +-- mobile/android/base/WebApp.java.in | 15 --------- .../base/WebAppManifestFragment.xml.frag | 12 +++++-- mobile/android/base/WebApps.java.in | 2 +- mobile/android/base/moz.build | 4 +-- 8 files changed, 49 insertions(+), 53 deletions(-) delete mode 100644 mobile/android/base/App.java.in rename mobile/android/base/{WebAppImpl.java => WebApp.java} (98%) delete mode 100644 mobile/android/base/WebApp.java.in diff --git a/mobile/android/base/AndroidManifest.xml.in b/mobile/android/base/AndroidManifest.xml.in index 1e5c921cb2e0..0204eeed07e4 100644 --- a/mobile/android/base/AndroidManifest.xml.in +++ b/mobile/android/base/AndroidManifest.xml.in @@ -82,12 +82,26 @@ android:debuggable="true"> #endif - + + + + @@ -153,9 +167,10 @@ - + - + + + + + - + diff --git a/mobile/android/base/App.java.in b/mobile/android/base/App.java.in deleted file mode 100644 index e58f006b22f9..000000000000 --- a/mobile/android/base/App.java.in +++ /dev/null @@ -1,15 +0,0 @@ -/* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*- - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -#filter substitution -package @ANDROID_PACKAGE_NAME@; - -import org.mozilla.gecko.BrowserApp; - -/** - * This class serves only as a namespace wrapper for BrowserApp. - */ -public class App extends BrowserApp {} - diff --git a/mobile/android/base/BrowserApp.java b/mobile/android/base/BrowserApp.java index 23578fecb666..0649c9cebe58 100644 --- a/mobile/android/base/BrowserApp.java +++ b/mobile/android/base/BrowserApp.java @@ -76,15 +76,15 @@ import java.net.URLEncoder; import java.util.EnumSet; import java.util.Vector; -abstract public class BrowserApp extends GeckoApp - implements TabsPanel.TabsLayoutChangeListener, - PropertyAnimator.PropertyAnimationListener, - View.OnKeyListener, - GeckoLayerClient.OnMetricsChangedListener, - BrowserSearch.OnSearchListener, - BrowserSearch.OnEditSuggestionListener, - HomePager.OnNewTabsListener, - OnUrlOpenListener { +public class BrowserApp extends GeckoApp + implements TabsPanel.TabsLayoutChangeListener, + PropertyAnimator.PropertyAnimationListener, + View.OnKeyListener, + GeckoLayerClient.OnMetricsChangedListener, + BrowserSearch.OnSearchListener, + BrowserSearch.OnEditSuggestionListener, + HomePager.OnNewTabsListener, + OnUrlOpenListener { private static final String LOGTAG = "GeckoBrowserApp"; private static final String PREF_CHROME_DYNAMICTOOLBAR = "browser.chrome.dynamictoolbar"; diff --git a/mobile/android/base/WebAppImpl.java b/mobile/android/base/WebApp.java similarity index 98% rename from mobile/android/base/WebAppImpl.java rename to mobile/android/base/WebApp.java index 5254605a7159..526b1049e805 100644 --- a/mobile/android/base/WebAppImpl.java +++ b/mobile/android/base/WebApp.java @@ -26,8 +26,8 @@ import android.view.Display; import java.net.URL; import java.io.File; -public class WebAppImpl extends GeckoApp { - private static final String LOGTAG = "GeckoWebAppImpl"; +public class WebApp extends GeckoApp { + private static final String LOGTAG = "GeckoWebApp"; private URL mOrigin; private TextView mTitlebarText = null; diff --git a/mobile/android/base/WebApp.java.in b/mobile/android/base/WebApp.java.in deleted file mode 100644 index 392257f4696b..000000000000 --- a/mobile/android/base/WebApp.java.in +++ /dev/null @@ -1,15 +0,0 @@ -/* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*- - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -#filter substitution -package @ANDROID_PACKAGE_NAME@; - -import org.mozilla.gecko.WebAppImpl; - -/** - * This class serves only as a namespace wrapper for WebAppImpl. - */ -public class WebApp extends WebAppImpl {} - diff --git a/mobile/android/base/WebAppManifestFragment.xml.frag b/mobile/android/base/WebAppManifestFragment.xml.frag index 3dc2390e632d..98847359c28f 100644 --- a/mobile/android/base/WebAppManifestFragment.xml.frag +++ b/mobile/android/base/WebAppManifestFragment.xml.frag @@ -1,4 +1,4 @@ - + + + + - - + diff --git a/mobile/android/base/WebApps.java.in b/mobile/android/base/WebApps.java.in index b36a1b73c21b..f6eba550fd7b 100644 --- a/mobile/android/base/WebApps.java.in +++ b/mobile/android/base/WebApps.java.in @@ -4,7 +4,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ #filter substitution -package @ANDROID_PACKAGE_NAME@; +package org.mozilla.gecko; /** * Declare a predefined number of WebApp classes to the WebApps class. diff --git a/mobile/android/base/moz.build b/mobile/android/base/moz.build index a50ca5851d36..4080014ecd7e 100644 --- a/mobile/android/base/moz.build +++ b/mobile/android/base/moz.build @@ -202,7 +202,7 @@ gbjar.sources += [ 'TouchEventInterceptor.java', 'VideoPlayer.java', 'WebAppAllocator.java', - 'WebAppImpl.java', + 'WebApp.java', 'ZoomConstraints.java', 'db/BrowserContract.java', 'db/BrowserProvider.java', @@ -317,11 +317,9 @@ gbjar.sources += [ thirdparty_source_dir + f for f in [ 'com/googlecode/eyesfree/braille/selfbraille/WriteData.java', ] ] gbjar.generated_sources += [ - 'App.java', 'AppConstants.java', 'R.java', 'SysInfo.java', - 'WebApp.java', 'WebApps.java', 'widget/GeckoEditText.java', 'widget/GeckoImageButton.java', From 0befe34a3a2f201aef61ec186b195aa69ebfa722 Mon Sep 17 00:00:00 2001 From: Brian Nicholson Date: Tue, 29 Oct 2013 12:33:47 -0700 Subject: [PATCH 18/24] Backed out changeset 8ebfd8ca2b65 for robocop failures. CLOSED TREE --HG-- rename : mobile/android/base/WebApp.java => mobile/android/base/WebAppImpl.java --- mobile/android/base/AndroidManifest.xml.in | 32 +++---------------- mobile/android/base/App.java.in | 15 +++++++++ mobile/android/base/BrowserApp.java | 18 +++++------ mobile/android/base/WebApp.java.in | 15 +++++++++ .../base/{WebApp.java => WebAppImpl.java} | 4 +-- .../base/WebAppManifestFragment.xml.frag | 12 ++----- mobile/android/base/WebApps.java.in | 2 +- mobile/android/base/moz.build | 4 ++- 8 files changed, 53 insertions(+), 49 deletions(-) create mode 100644 mobile/android/base/App.java.in create mode 100644 mobile/android/base/WebApp.java.in rename mobile/android/base/{WebApp.java => WebAppImpl.java} (98%) diff --git a/mobile/android/base/AndroidManifest.xml.in b/mobile/android/base/AndroidManifest.xml.in index 0204eeed07e4..1e5c921cb2e0 100644 --- a/mobile/android/base/AndroidManifest.xml.in +++ b/mobile/android/base/AndroidManifest.xml.in @@ -82,26 +82,12 @@ android:debuggable="true"> #endif - - - - - @@ -167,10 +153,9 @@ - + - - - - - - - + diff --git a/mobile/android/base/App.java.in b/mobile/android/base/App.java.in new file mode 100644 index 000000000000..e58f006b22f9 --- /dev/null +++ b/mobile/android/base/App.java.in @@ -0,0 +1,15 @@ +/* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*- + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#filter substitution +package @ANDROID_PACKAGE_NAME@; + +import org.mozilla.gecko.BrowserApp; + +/** + * This class serves only as a namespace wrapper for BrowserApp. + */ +public class App extends BrowserApp {} + diff --git a/mobile/android/base/BrowserApp.java b/mobile/android/base/BrowserApp.java index 0649c9cebe58..23578fecb666 100644 --- a/mobile/android/base/BrowserApp.java +++ b/mobile/android/base/BrowserApp.java @@ -76,15 +76,15 @@ import java.net.URLEncoder; import java.util.EnumSet; import java.util.Vector; -public class BrowserApp extends GeckoApp - implements TabsPanel.TabsLayoutChangeListener, - PropertyAnimator.PropertyAnimationListener, - View.OnKeyListener, - GeckoLayerClient.OnMetricsChangedListener, - BrowserSearch.OnSearchListener, - BrowserSearch.OnEditSuggestionListener, - HomePager.OnNewTabsListener, - OnUrlOpenListener { +abstract public class BrowserApp extends GeckoApp + implements TabsPanel.TabsLayoutChangeListener, + PropertyAnimator.PropertyAnimationListener, + View.OnKeyListener, + GeckoLayerClient.OnMetricsChangedListener, + BrowserSearch.OnSearchListener, + BrowserSearch.OnEditSuggestionListener, + HomePager.OnNewTabsListener, + OnUrlOpenListener { private static final String LOGTAG = "GeckoBrowserApp"; private static final String PREF_CHROME_DYNAMICTOOLBAR = "browser.chrome.dynamictoolbar"; diff --git a/mobile/android/base/WebApp.java.in b/mobile/android/base/WebApp.java.in new file mode 100644 index 000000000000..392257f4696b --- /dev/null +++ b/mobile/android/base/WebApp.java.in @@ -0,0 +1,15 @@ +/* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*- + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#filter substitution +package @ANDROID_PACKAGE_NAME@; + +import org.mozilla.gecko.WebAppImpl; + +/** + * This class serves only as a namespace wrapper for WebAppImpl. + */ +public class WebApp extends WebAppImpl {} + diff --git a/mobile/android/base/WebApp.java b/mobile/android/base/WebAppImpl.java similarity index 98% rename from mobile/android/base/WebApp.java rename to mobile/android/base/WebAppImpl.java index 526b1049e805..5254605a7159 100644 --- a/mobile/android/base/WebApp.java +++ b/mobile/android/base/WebAppImpl.java @@ -26,8 +26,8 @@ import android.view.Display; import java.net.URL; import java.io.File; -public class WebApp extends GeckoApp { - private static final String LOGTAG = "GeckoWebApp"; +public class WebAppImpl extends GeckoApp { + private static final String LOGTAG = "GeckoWebAppImpl"; private URL mOrigin; private TextView mTitlebarText = null; diff --git a/mobile/android/base/WebAppManifestFragment.xml.frag b/mobile/android/base/WebAppManifestFragment.xml.frag index 98847359c28f..3dc2390e632d 100644 --- a/mobile/android/base/WebAppManifestFragment.xml.frag +++ b/mobile/android/base/WebAppManifestFragment.xml.frag @@ -1,4 +1,4 @@ - - - - - - + + diff --git a/mobile/android/base/WebApps.java.in b/mobile/android/base/WebApps.java.in index f6eba550fd7b..b36a1b73c21b 100644 --- a/mobile/android/base/WebApps.java.in +++ b/mobile/android/base/WebApps.java.in @@ -4,7 +4,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ #filter substitution -package org.mozilla.gecko; +package @ANDROID_PACKAGE_NAME@; /** * Declare a predefined number of WebApp classes to the WebApps class. diff --git a/mobile/android/base/moz.build b/mobile/android/base/moz.build index 4080014ecd7e..a50ca5851d36 100644 --- a/mobile/android/base/moz.build +++ b/mobile/android/base/moz.build @@ -202,7 +202,7 @@ gbjar.sources += [ 'TouchEventInterceptor.java', 'VideoPlayer.java', 'WebAppAllocator.java', - 'WebApp.java', + 'WebAppImpl.java', 'ZoomConstraints.java', 'db/BrowserContract.java', 'db/BrowserProvider.java', @@ -317,9 +317,11 @@ gbjar.sources += [ thirdparty_source_dir + f for f in [ 'com/googlecode/eyesfree/braille/selfbraille/WriteData.java', ] ] gbjar.generated_sources += [ + 'App.java', 'AppConstants.java', 'R.java', 'SysInfo.java', + 'WebApp.java', 'WebApps.java', 'widget/GeckoEditText.java', 'widget/GeckoImageButton.java', From 97b12480eb84ce4d7f2a7379195593c67fa32121 Mon Sep 17 00:00:00 2001 From: Richard Newman Date: Tue, 29 Oct 2013 15:00:35 -0700 Subject: [PATCH 19/24] Bug 925521 - Fix yet another test with a hardcoded version. r=me --- .../content/test/general/browser_urlbar_search_healthreport.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/browser/base/content/test/general/browser_urlbar_search_healthreport.js b/browser/base/content/test/general/browser_urlbar_search_healthreport.js index 4315a98649ed..ade93ea4f551 100644 --- a/browser/base/content/test/general/browser_urlbar_search_healthreport.js +++ b/browser/base/content/test/general/browser_urlbar_search_healthreport.js @@ -23,7 +23,7 @@ function test() { reporter.onInit().then(function onInit() { let provider = reporter.getProvider("org.mozilla.searches"); ok(provider, "Searches provider is available."); - let m = provider.getMeasurement("counts", 2); + let m = provider.getMeasurement("counts", 3); m.getValues().then(function onData(data) { let now = new Date(); From 731d75722bc634c0c292042c063b1a4c2c447692 Mon Sep 17 00:00:00 2001 From: Jim Mathies Date: Tue, 29 Oct 2013 20:51:00 -0500 Subject: [PATCH 20/24] Bug 931146 - Set FrameMetrics mMayHaveTouchListeners for all frames that call RecordFrameMetrics. Fixes missing touch info for sub documents. r=tnikkel,kats --- layout/base/nsDisplayList.cpp | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/layout/base/nsDisplayList.cpp b/layout/base/nsDisplayList.cpp index 06f43e9e06a2..958f93222be3 100644 --- a/layout/base/nsDisplayList.cpp +++ b/layout/base/nsDisplayList.cpp @@ -643,8 +643,7 @@ static void RecordFrameMetrics(nsIFrame* aForFrame, nsRect* aDisplayPort, nsRect* aCriticalDisplayPort, ViewID aScrollId, - const nsDisplayItem::ContainerParameters& aContainerParameters, - bool aMayHaveTouchListeners) { + const nsDisplayItem::ContainerParameters& aContainerParameters) { nsPresContext* presContext = aForFrame->PresContext(); int32_t auPerDevPixel = presContext->AppUnitsPerDevPixel(); LayoutDeviceToLayerScale resolution(aContainerParameters.mXScale, aContainerParameters.mYScale); @@ -719,7 +718,16 @@ static void RecordFrameMetrics(nsIFrame* aForFrame, metrics.mZoom = metrics.mCumulativeResolution * metrics.mDevPixelsPerCSSPixel * layerToScreenScale; - metrics.mMayHaveTouchListeners = aMayHaveTouchListeners; + if (presShell) { + nsIDocument* document = nullptr; + document = presShell->GetDocument(); + if (document) { + nsCOMPtr innerWin(document->GetInnerWindow()); + if (innerWin) { + metrics.mMayHaveTouchListeners = innerWin->HasTouchEventListeners(); + } + } + } // Calculate the composition bounds as the size of the scroll frame and // its origin relative to the reference frame. @@ -1248,14 +1256,6 @@ void nsDisplayList::PaintForFrame(nsDisplayListBuilder* aBuilder, } } - bool mayHaveTouchListeners = false; - if (document) { - nsCOMPtr innerWin(document->GetInnerWindow()); - if (innerWin) { - mayHaveTouchListeners = innerWin->HasTouchEventListeners(); - } - } - nsRect viewport(aBuilder->ToReferenceFrame(aForFrame), aForFrame->GetSize()); RecordFrameMetrics(aForFrame, rootScrollFrame, @@ -1263,7 +1263,7 @@ void nsDisplayList::PaintForFrame(nsDisplayListBuilder* aBuilder, root, mVisibleRect, viewport, (usingDisplayport ? &displayport : nullptr), (usingCriticalDisplayport ? &criticalDisplayport : nullptr), - id, containerParameters, mayHaveTouchListeners); + id, containerParameters); if (usingDisplayport && !(root->GetContentFlags() & Layer::CONTENT_OPAQUE)) { // See bug 693938, attachment 567017 @@ -3576,7 +3576,7 @@ nsDisplayScrollLayer::BuildLayer(nsDisplayListBuilder* aBuilder, mVisibleRect, viewport, (usingDisplayport ? &displayport : nullptr), (usingCriticalDisplayport ? &criticalDisplayport : nullptr), - scrollId, aContainerParameters, false); + scrollId, aContainerParameters); return layer.forget(); } From 1b4c5c51056c7b99c9c1cbb8e936f00718ed4080 Mon Sep 17 00:00:00 2001 From: Jim Mathies Date: Tue, 29 Oct 2013 20:51:00 -0500 Subject: [PATCH 21/24] Bug 931504 - Fire touchcancel to content for every touch point the apz consumes. r=tabraldes --- widget/windows/winrt/MetroInput.cpp | 42 ++++++++++++++++++++--------- widget/windows/winrt/MetroInput.h | 4 +-- 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/widget/windows/winrt/MetroInput.cpp b/widget/windows/winrt/MetroInput.cpp index 3e4396aa4b80..a5e8f02710d6 100644 --- a/widget/windows/winrt/MetroInput.cpp +++ b/widget/windows/winrt/MetroInput.cpp @@ -487,7 +487,7 @@ MetroInput::OnPointerPressed(UI::Core::ICoreWindow* aSender, mTouchMoveDefaultPrevented = false; mIsFirstTouchMove = true; mCancelable = true; - mTouchCancelSent = false; + mCanceledIds.Clear(); InitTouchEventTouchList(touchEvent); DispatchAsyncTouchEventWithCallback(touchEvent, &MetroInput::OnPointerPressedCallback); } else { @@ -1217,6 +1217,10 @@ MetroInput::DeliverNextQueuedTouchEvent() * a touchcancel to content and do not deliver any additional events there. * (If the apz is doing something with the events we can save ourselves * the overhead of delivering dom events.) + * + * Notes: + * - never rely on the contents of mTouches here, since this is a delayed + * callback. mTouches will likely have been modified. */ // Test for chrome vs. content target. To do this we only use the first touch @@ -1247,10 +1251,7 @@ MetroInput::DeliverNextQueuedTouchEvent() WidgetTouchEvent transformedEvent(*event); status = mWidget->ApzReceiveInputEvent(event, &transformedEvent); if (!mCancelable && status == nsEventStatus_eConsumeNoDefault) { - if (!mTouchCancelSent) { - mTouchCancelSent = true; - DispatchTouchCancel(); - } + DispatchTouchCancel(event); return status; } @@ -1261,16 +1262,31 @@ MetroInput::DeliverNextQueuedTouchEvent() } void -MetroInput::DispatchTouchCancel() +MetroInput::DispatchTouchCancel(WidgetTouchEvent* aEvent) { - LogFunction(); - // From the spec: The touch point or points that were removed must be - // included in the changedTouches attribute of the TouchEvent, and must - // not be included in the touches and targetTouches attributes. - // (We are 'removing' all touch points that have been sent to content - // thus far.) + MOZ_ASSERT(aEvent); + // Send a touchcancel for each pointer id we have a corresponding start + // for. Note we can't rely on mTouches here since touchends remove points + // from it. The only time we end up in here is if the apz is consuming + // events, so this array shouldn't be very large. WidgetTouchEvent touchEvent(true, NS_TOUCH_CANCEL, mWidget.Get()); - InitTouchEventTouchList(&touchEvent); + nsTArray< nsRefPtr >& touches = aEvent->touches; + for (uint32_t i = 0; i < touches.Length(); ++i) { + dom::Touch* touch = touches[i]; + if (!touch) { + continue; + } + int32_t id = touch->Identifier(); + if (mCanceledIds.Contains(id)) { + continue; + } + mCanceledIds.AppendElement(id); + touchEvent.touches.AppendElement(touch); + } + if (!touchEvent.touches.Length()) { + return; + } + mWidget->DispatchEvent(&touchEvent, sThrowawayStatus); } diff --git a/widget/windows/winrt/MetroInput.h b/widget/windows/winrt/MetroInput.h index f676b384263a..c156db3df99a 100644 --- a/widget/windows/winrt/MetroInput.h +++ b/widget/windows/winrt/MetroInput.h @@ -211,7 +211,7 @@ private: bool mTouchMoveDefaultPrevented; bool mIsFirstTouchMove; bool mCancelable; - bool mTouchCancelSent; + nsTArray mCanceledIds; // In the old Win32 way of doing things, we would receive a WM_TOUCH event // that told us the state of every touchpoint on the touch surface. If @@ -287,7 +287,7 @@ private: // Sync event dispatching void DispatchEventIgnoreStatus(WidgetGUIEvent* aEvent); - void DispatchTouchCancel(); + void DispatchTouchCancel(WidgetTouchEvent* aEvent); nsDeque mInputEventQueue; static nsEventStatus sThrowawayStatus; From 6c63a152ba7fe654453a26b84c4a62592c5bcb9e Mon Sep 17 00:00:00 2001 From: Jim Mathies Date: Tue, 29 Oct 2013 20:51:00 -0500 Subject: [PATCH 22/24] Bug 918273 - Fix: ContentReceivedTouch doesn't actually cancel event processing in the apzc. r=kats --- gfx/layers/ipc/AsyncPanZoomController.cpp | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/gfx/layers/ipc/AsyncPanZoomController.cpp b/gfx/layers/ipc/AsyncPanZoomController.cpp index a82b0b299a96..9b3ca59f1337 100644 --- a/gfx/layers/ipc/AsyncPanZoomController.cpp +++ b/gfx/layers/ipc/AsyncPanZoomController.cpp @@ -369,6 +369,9 @@ nsEventStatus AsyncPanZoomController::ReceiveInputEvent(const InputData& aEvent) (mState == NOTHING || mState == TOUCHING || IsPanningState(mState))) { const MultiTouchInput& multiTouchInput = aEvent.AsMultiTouchInput(); if (multiTouchInput.mType == MultiTouchInput::MULTITOUCH_START) { + // Wait for a set timeout or preventDefault info via ContentReceivedTouch + // before interacting with content. In the mean time we queue up events + // for processing later. SetState(WAITING_LISTENERS); } } @@ -404,7 +407,7 @@ nsEventStatus AsyncPanZoomController::HandleInputEvent(const InputData& aEvent) if (mDelayPanning && aEvent.mInputType == MULTITOUCH_INPUT) { const MultiTouchInput& multiTouchInput = aEvent.AsMultiTouchInput(); if (multiTouchInput.mType == MultiTouchInput::MULTITOUCH_MOVE) { - // Let BrowserElementScrolling perform panning gesture first. + // Wait for a set timeout or preventDefault info via ContentReceivedTouch. SetState(WAITING_LISTENERS); mTouchQueue.AppendElement(multiTouchInput); @@ -1366,6 +1369,9 @@ void AsyncPanZoomController::UpdateCompositionBounds(const ScreenIntRect& aCompo void AsyncPanZoomController::CancelDefaultPanZoom() { mDisableNextTouchBatch = true; + mDelayPanning = false; + mTouchQueue.Clear(); + SetState(NOTHING); nsRefPtr listener = GetGestureEventListener(); if (listener) { listener->CancelGesture(); @@ -1466,7 +1472,12 @@ void AsyncPanZoomController::ContentReceivedTouch(bool aPreventDefault) { } if (mState == WAITING_LISTENERS) { - if (!aPreventDefault) { + if (aPreventDefault) { + // We're being told that this touch block will be consumed by content, + // reset state and block processing until the next touch block. + CancelDefaultPanZoom(); + return; + } else { // Delayed scrolling gesture is pending at TOUCHING state. if (mDelayPanning) { SetState(TOUCHING); From 8d45d7528959dcdf4746b56a2926fe5c1646adfa Mon Sep 17 00:00:00 2001 From: Jim Mathies Date: Tue, 29 Oct 2013 20:51:00 -0500 Subject: [PATCH 23/24] Bug 931763 - Simplify handling of first touch start and touch move in MetroInput. r=tabraldes --- widget/windows/winrt/MetroInput.cpp | 191 +++++++++++++--------------- widget/windows/winrt/MetroInput.h | 15 +-- 2 files changed, 93 insertions(+), 113 deletions(-) diff --git a/widget/windows/winrt/MetroInput.cpp b/widget/windows/winrt/MetroInput.cpp index a5e8f02710d6..8df22deb9f6e 100644 --- a/widget/windows/winrt/MetroInput.cpp +++ b/widget/windows/winrt/MetroInput.cpp @@ -440,6 +440,16 @@ MetroInput::InitTouchEventTouchList(WidgetTouchEvent* aEvent) static_cast(&aEvent->touches)); } +bool +MetroInput::ShouldDeliverInputToRecognizer() +{ + // If the event is destined for chrome deliver all events to the recognizer. + if (mChromeHitTestCacheForTouch) { + return true; + } + return mRecognizerWantsEvents; +} + // This event is raised when the user pushes the left mouse button, presses a // pen to the surface, or presses a touch screen. HRESULT @@ -483,44 +493,26 @@ MetroInput::OnPointerPressed(UI::Core::ICoreWindow* aSender, // If this is the first touchstart of a touch session reset some // tracking flags and dispatch the event with a custom callback // so we can check preventDefault result. - mTouchStartDefaultPrevented = false; - mTouchMoveDefaultPrevented = false; + mContentConsumingTouch = false; + mRecognizerWantsEvents = true; mIsFirstTouchMove = true; mCancelable = true; mCanceledIds.Clear(); - InitTouchEventTouchList(touchEvent); - DispatchAsyncTouchEventWithCallback(touchEvent, &MetroInput::OnPointerPressedCallback); - } else { - InitTouchEventTouchList(touchEvent); - DispatchAsyncTouchEventIgnoreStatus(touchEvent); } - if (!mTouchStartDefaultPrevented) { + InitTouchEventTouchList(touchEvent); + DispatchAsyncTouchEvent(touchEvent); + + if (ShouldDeliverInputToRecognizer()) { mGestureRecognizer->ProcessDownEvent(currentPoint.Get()); } return S_OK; } -void -MetroInput::OnPointerPressedCallback() -{ - nsEventStatus status = DeliverNextQueuedTouchEvent(); - mTouchStartDefaultPrevented = (nsEventStatus_eConsumeNoDefault == status); - if (mTouchStartDefaultPrevented) { - // If content canceled the first touchstart don't generate any gesture based - // input - clear the recognizer state without sending any events. - mGestureRecognizer->CompleteGesture(); - // Let the apz know content wants to consume touch events. - mWidget->ApzContentConsumingTouch(); - } -} - void MetroInput::AddPointerMoveDataToRecognizer(UI::Core::IPointerEventArgs* aArgs) { - // Only feed move input to the recognizer if the first touchstart and - // subsequent touchmove return results were not eConsumeNoDefault. - if (!mTouchStartDefaultPrevented && !mTouchMoveDefaultPrevented) { + if (ShouldDeliverInputToRecognizer()) { WRL::ComPtr> pointerPoints; aArgs->GetIntermediatePoints(pointerPoints.GetAddressOf()); @@ -586,7 +578,7 @@ MetroInput::OnPointerMoved(UI::Core::ICoreWindow* aSender, WidgetTouchEvent* touchEvent = new WidgetTouchEvent(true, NS_TOUCH_MOVE, mWidget.Get()); InitTouchEventTouchList(touchEvent); - DispatchAsyncTouchEventIgnoreStatus(touchEvent); + DispatchAsyncTouchEvent(touchEvent); } touch = CreateDOMTouch(currentPoint.Get()); @@ -597,13 +589,10 @@ MetroInput::OnPointerMoved(UI::Core::ICoreWindow* aSender, WidgetTouchEvent* touchEvent = new WidgetTouchEvent(true, NS_TOUCH_MOVE, mWidget.Get()); - // If this is the first touch move of our session, we should check the result. - // Note we may lose some touch move data here for the recognizer since we want - // to wait until we have the result of the first touchmove dispatch. For gesture - // based events this shouldn't break anything. + // If this is the first touch move of our session, dispatch it now. if (mIsFirstTouchMove) { InitTouchEventTouchList(touchEvent); - DispatchAsyncTouchEventWithCallback(touchEvent, &MetroInput::OnFirstPointerMoveCallback); + DispatchAsyncTouchEvent(touchEvent); mIsFirstTouchMove = false; } @@ -612,22 +601,6 @@ MetroInput::OnPointerMoved(UI::Core::ICoreWindow* aSender, return S_OK; } -void -MetroInput::OnFirstPointerMoveCallback() -{ - nsEventStatus status = DeliverNextQueuedTouchEvent(); - mCancelable = false; - mTouchMoveDefaultPrevented = (nsEventStatus_eConsumeNoDefault == status); - // Let the apz know whether content wants to consume touch events - if (mTouchMoveDefaultPrevented) { - mWidget->ApzContentConsumingTouch(); - // reset the recognizer - mGestureRecognizer->CompleteGesture(); - } else if (!mTouchMoveDefaultPrevented && !mTouchStartDefaultPrevented) { - mWidget->ApzContentIgnoringTouch(); - } -} - // This event is raised when the user lifts the left mouse button, lifts a // pen from the surface, or lifts her/his finger from a touch screen. HRESULT @@ -667,7 +640,7 @@ MetroInput::OnPointerReleased(UI::Core::ICoreWindow* aSender, WidgetTouchEvent* touchEvent = new WidgetTouchEvent(true, NS_TOUCH_MOVE, mWidget.Get()); InitTouchEventTouchList(touchEvent); - DispatchAsyncTouchEventIgnoreStatus(touchEvent); + DispatchAsyncTouchEvent(touchEvent); } // Remove this touch point from our map. Eventually all touch points are @@ -679,11 +652,9 @@ MetroInput::OnPointerReleased(UI::Core::ICoreWindow* aSender, WidgetTouchEvent* touchEvent = new WidgetTouchEvent(true, NS_TOUCH_END, mWidget.Get()); touchEvent->touches.AppendElement(CreateDOMTouch(currentPoint.Get())); - DispatchAsyncTouchEventIgnoreStatus(touchEvent); + DispatchAsyncTouchEvent(touchEvent); - // If content didn't cancel the first touchstart feed touchend data to the - // recognizer. - if (!mTouchStartDefaultPrevented && !mTouchMoveDefaultPrevented) { + if (ShouldDeliverInputToRecognizer()) { mGestureRecognizer->ProcessUpEvent(currentPoint.Get()); } @@ -1179,7 +1150,7 @@ MetroInput::DeliverNextQueuedEventIgnoreStatus() } void -MetroInput::DispatchAsyncTouchEventIgnoreStatus(WidgetTouchEvent* aEvent) +MetroInput::DispatchAsyncTouchEvent(WidgetTouchEvent* aEvent) { aEvent->time = ::GetMessageTime(); mModifierKeyState.Update(); @@ -1190,7 +1161,7 @@ MetroInput::DispatchAsyncTouchEventIgnoreStatus(WidgetTouchEvent* aEvent) NS_DispatchToCurrentThread(runnable); } -nsEventStatus +void MetroInput::DeliverNextQueuedTouchEvent() { nsEventStatus status; @@ -1203,20 +1174,18 @@ MetroInput::DeliverNextQueuedTouchEvent() /* * We go through states here and make different decisions in each: * - * 1) delivering first touchpoint touchstart or its first touchmove - * Our callers (OnFirstPointerMoveCallback, OnPointerPressedCallback) will - * check our result and set mTouchStartDefaultPrevented or - * mTouchMoveDefaultPrevented appropriately. Deliver touch events to the apz - * (ignoring return result) and to content and return the content event - * status result to our caller. - * 2) mTouchStartDefaultPrevented or mTouchMoveDefaultPrevented are true - * Deliver touch to content after transforming through the apz. Our callers - * handle calling cancel for the touch sequence on the apz. - * 3) mTouchStartDefaultPrevented and mTouchMoveDefaultPrevented are false - * Deliver events to the apz. If the apz returns eConsumeNoDefault dispatch - * a touchcancel to content and do not deliver any additional events there. - * (If the apz is doing something with the events we can save ourselves - * the overhead of delivering dom events.) + * 1) Hit test chrome on first touchstart + * If chrome is the target simplify event delivery from that point + * on by directing all input to chrome, bypassing the apz. + * 2) Process first touchpoint touchstart and touchmove + * Check the result and set mContentConsumingTouch appropriately. Deliver + * touch events to the apz (ignoring return result) and to content. + * 3) If mContentConsumingTouch is true: deliver touch to content after + * transforming through the apz. Also let the apz know content is + * consuming touch. + * 4) If mContentConsumingTouch is false: send a touchcancel to content + * and deliver all events to the apz. If the apz is doing something with + * the events we can save ourselves the overhead of delivering dom events. * * Notes: * - never rely on the contents of mTouches here, since this is a delayed @@ -1226,39 +1195,68 @@ MetroInput::DeliverNextQueuedTouchEvent() // Test for chrome vs. content target. To do this we only use the first touch // point since that will be the input batch target. Cache this for touch events // since HitTestChrome has to send a dom event. - if (event->message == NS_TOUCH_START) { + if (mCancelable && event->message == NS_TOUCH_START) { nsRefPtr touch = event->touches[0]; LayoutDeviceIntPoint pt = LayoutDeviceIntPoint::FromUntyped(touch->mRefPoint); bool apzIntersect = mWidget->HitTestAPZC(mozilla::ScreenPoint(pt.x, pt.y)); mChromeHitTestCacheForTouch = (apzIntersect && HitTestChrome(pt)); } - // Check if content called preventDefault on touchstart or first touchmove. If so - // and the event is destined for chrome, send the event. If destined for content, - // translate coordinates through the apz then send. - if (mTouchStartDefaultPrevented || mTouchMoveDefaultPrevented) { - if (!mChromeHitTestCacheForTouch) { - // ContentReceivedTouch has already been called so this shouldn't cause - // the apz to react. We still need to transform our coordinates though. - mWidget->ApzReceiveInputEvent(event); - } + // If this event is destined for chrome, deliver it directly there bypassing + // the apz. + if (!mCancelable && mChromeHitTestCacheForTouch) { mWidget->DispatchEvent(event, status); - return status; + return; } - // Forward event data to apz. If the apz consumes the event, don't forward to - // content if this is not a cancelable event. - WidgetTouchEvent transformedEvent(*event); - status = mWidget->ApzReceiveInputEvent(event, &transformedEvent); - if (!mCancelable && status == nsEventStatus_eConsumeNoDefault) { - DispatchTouchCancel(event); - return status; + // If we have yet to deliver the first touch start and touch move, deliver the + // event to both content and the apz. Ignore the apz's return result since we + // give content the option of saying it wants to consume touch for both events. + if (mCancelable) { + WidgetTouchEvent transformedEvent(*event); + mWidget->ApzReceiveInputEvent(event, &transformedEvent); + mWidget->DispatchEvent(mChromeHitTestCacheForTouch ? event : &transformedEvent, status); + if (event->message == NS_TOUCH_START) { + mContentConsumingTouch = (nsEventStatus_eConsumeNoDefault == status); + // Disable gesture based events (taps, swipes, rotation) if + // preventDefault is called on touchstart. + mRecognizerWantsEvents = !(nsEventStatus_eConsumeNoDefault == status); + } else if (event->message == NS_TOUCH_MOVE) { + mCancelable = false; + // Add this result to to our content comsuming flag + if (!mContentConsumingTouch) { + mContentConsumingTouch = (nsEventStatus_eConsumeNoDefault == status); + } + // Let the apz know if content wants to consume touch events, or cancel + // the touch block for content. + if (mContentConsumingTouch) { + mWidget->ApzContentConsumingTouch(); + } else { + mWidget->ApzContentIgnoringTouch(); + DispatchTouchCancel(&transformedEvent); + } + } + // If content is consuming touch don't generate any gesture based + // input - clear the recognizer state without sending any events. + if (!ShouldDeliverInputToRecognizer()) { + mGestureRecognizer->CompleteGesture(); + } + return; } - // Deliver event. If this is destined for chrome, use the untransformed event - // data, if it's destined for content, use the transformed event. - mWidget->DispatchEvent(!mChromeHitTestCacheForTouch ? &transformedEvent : event, status); - return status; + // If content called preventDefault on touchstart or first touchmove send + // the event to content. + if (mContentConsumingTouch) { + // ContentReceivedTouch has already been called in the mCancelable block + // above so this shouldn't cause the apz to react. We still need to + // transform our coordinates though. + mWidget->ApzReceiveInputEvent(event); + mWidget->DispatchEvent(event, status); + return; + } + + // Forward event data to apz. + mWidget->ApzReceiveInputEvent(event); } void @@ -1290,19 +1288,6 @@ MetroInput::DispatchTouchCancel(WidgetTouchEvent* aEvent) mWidget->DispatchEvent(&touchEvent, sThrowawayStatus); } -void -MetroInput::DispatchAsyncTouchEventWithCallback(WidgetTouchEvent* aEvent, - void (MetroInput::*Callback)()) -{ - aEvent->time = ::GetMessageTime(); - mModifierKeyState.Update(); - mModifierKeyState.InitInputEvent(*aEvent); - mInputEventQueue.Push(aEvent); - nsCOMPtr runnable = - NS_NewRunnableMethod(this, Callback); - NS_DispatchToCurrentThread(runnable); -} - void MetroInput::DispatchEventIgnoreStatus(WidgetGUIEvent *aEvent) { diff --git a/widget/windows/winrt/MetroInput.h b/widget/windows/winrt/MetroInput.h index c156db3df99a..0c76184241da 100644 --- a/widget/windows/winrt/MetroInput.h +++ b/widget/windows/winrt/MetroInput.h @@ -183,6 +183,7 @@ private: uint32_t aMagEventType, uint32_t aRotEventType); uint16_t ProcessInputTypeForGesture(IEdgeGestureEventArgs* aArgs); + bool ShouldDeliverInputToRecognizer(); // The W3C spec states that "whether preventDefault has been called" should // be tracked on a per-touchpoint basis, but it also states that touchstart @@ -207,10 +208,10 @@ private: // events will be generated based on the touchstart and touchend events. // For example, a set of mousemove, mousedown, and mouseup events might // be sent if a tap is detected. - bool mTouchStartDefaultPrevented; - bool mTouchMoveDefaultPrevented; + bool mContentConsumingTouch; bool mIsFirstTouchMove; bool mCancelable; + bool mRecognizerWantsEvents; nsTArray mCanceledIds; // In the old Win32 way of doing things, we would receive a WM_TOUCH event @@ -273,17 +274,11 @@ private: // Async event dispatching void DispatchAsyncEventIgnoreStatus(WidgetInputEvent* aEvent); - void DispatchAsyncTouchEventIgnoreStatus(WidgetTouchEvent* aEvent); - void DispatchAsyncTouchEventWithCallback(WidgetTouchEvent* aEvent, - void (MetroInput::*Callback)()); + void DispatchAsyncTouchEvent(WidgetTouchEvent* aEvent); // Async event callbacks void DeliverNextQueuedEventIgnoreStatus(); - nsEventStatus DeliverNextQueuedTouchEvent(); - - // Misc. specialty async callbacks - void OnPointerPressedCallback(); - void OnFirstPointerMoveCallback(); + void DeliverNextQueuedTouchEvent(); // Sync event dispatching void DispatchEventIgnoreStatus(WidgetGUIEvent* aEvent); From ffe224c10174d3703f8ad086762a9acf379bc4dc Mon Sep 17 00:00:00 2001 From: Jim Mathies Date: Wed, 30 Oct 2013 16:54:50 -0500 Subject: [PATCH 24/24] Backed out changeset 0191d92d4f42 (bug 918273) because it conflicts with bug 906877 which just landed on b2g. r=RyanVM, CLOSED TREE --- gfx/layers/ipc/AsyncPanZoomController.cpp | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/gfx/layers/ipc/AsyncPanZoomController.cpp b/gfx/layers/ipc/AsyncPanZoomController.cpp index 9b3ca59f1337..a82b0b299a96 100644 --- a/gfx/layers/ipc/AsyncPanZoomController.cpp +++ b/gfx/layers/ipc/AsyncPanZoomController.cpp @@ -369,9 +369,6 @@ nsEventStatus AsyncPanZoomController::ReceiveInputEvent(const InputData& aEvent) (mState == NOTHING || mState == TOUCHING || IsPanningState(mState))) { const MultiTouchInput& multiTouchInput = aEvent.AsMultiTouchInput(); if (multiTouchInput.mType == MultiTouchInput::MULTITOUCH_START) { - // Wait for a set timeout or preventDefault info via ContentReceivedTouch - // before interacting with content. In the mean time we queue up events - // for processing later. SetState(WAITING_LISTENERS); } } @@ -407,7 +404,7 @@ nsEventStatus AsyncPanZoomController::HandleInputEvent(const InputData& aEvent) if (mDelayPanning && aEvent.mInputType == MULTITOUCH_INPUT) { const MultiTouchInput& multiTouchInput = aEvent.AsMultiTouchInput(); if (multiTouchInput.mType == MultiTouchInput::MULTITOUCH_MOVE) { - // Wait for a set timeout or preventDefault info via ContentReceivedTouch. + // Let BrowserElementScrolling perform panning gesture first. SetState(WAITING_LISTENERS); mTouchQueue.AppendElement(multiTouchInput); @@ -1369,9 +1366,6 @@ void AsyncPanZoomController::UpdateCompositionBounds(const ScreenIntRect& aCompo void AsyncPanZoomController::CancelDefaultPanZoom() { mDisableNextTouchBatch = true; - mDelayPanning = false; - mTouchQueue.Clear(); - SetState(NOTHING); nsRefPtr listener = GetGestureEventListener(); if (listener) { listener->CancelGesture(); @@ -1472,12 +1466,7 @@ void AsyncPanZoomController::ContentReceivedTouch(bool aPreventDefault) { } if (mState == WAITING_LISTENERS) { - if (aPreventDefault) { - // We're being told that this touch block will be consumed by content, - // reset state and block processing until the next touch block. - CancelDefaultPanZoom(); - return; - } else { + if (!aPreventDefault) { // Delayed scrolling gesture is pending at TOUCHING state. if (mDelayPanning) { SetState(TOUCHING);