From f74b021acab82d06aa3a435983d6da94e034a172 Mon Sep 17 00:00:00 2001 From: "Bernardo P. Rittmeyer" Date: Mon, 3 Aug 2015 15:16:08 -0700 Subject: [PATCH 01/28] Bug 107957 - Fixed sorting order for password manager autocomplete. r=MattN --- toolkit/components/passwordmgr/LoginManagerContent.jsm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/toolkit/components/passwordmgr/LoginManagerContent.jsm b/toolkit/components/passwordmgr/LoginManagerContent.jsm index b06b4fb0dfd..419bc1bec62 100644 --- a/toolkit/components/passwordmgr/LoginManagerContent.jsm +++ b/toolkit/components/passwordmgr/LoginManagerContent.jsm @@ -1070,7 +1070,7 @@ function UserAutoCompleteResult (aSearchString, matchingLogins) { if (userA < userB) return -1; - if (userB > userA) + if (userA > userB) return 1; return 0; From a40b7faa1b6ae634e50be8cda1cee1623f0645eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A3o=20Gottwald?= Date: Sun, 2 Aug 2015 12:50:00 -0400 Subject: [PATCH 02/28] Bug 1190209 - tabbox.xml should still set the visuallyselected attribute. r=mconley CLOSED TREE --- toolkit/content/widgets/tabbox.xml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/toolkit/content/widgets/tabbox.xml b/toolkit/content/widgets/tabbox.xml index 5ad0a8ce82f..927affd82de 100644 --- a/toolkit/content/widgets/tabbox.xml +++ b/toolkit/content/widgets/tabbox.xml @@ -729,10 +729,13 @@ Date: Wed, 22 Jul 2015 21:06:33 -0700 Subject: [PATCH 03/28] Bug 1184211 - Replace TwoWayView in SearchBar with RecyclerView. r=sebastian - Also includes changes from bug 1158291 applied on top of the re-written view. CLOSED TREE --- mobile/android/base/home/BrowserSearch.java | 1 - .../base/home/SearchEngineAdapter.java | 122 ++++++++++ mobile/android/base/home/SearchEngineBar.java | 209 ++++++------------ mobile/android/base/moz.build | 1 + .../layout/search_engine_bar_item.xml | 1 + .../layout/search_engine_bar_label.xml | 2 +- .../tests/browser/robocop/AboutHomeTest.java | 5 +- .../browser/robocop/testAddSearchEngine.java | 2 +- 8 files changed, 198 insertions(+), 145 deletions(-) create mode 100644 mobile/android/base/home/SearchEngineAdapter.java diff --git a/mobile/android/base/home/BrowserSearch.java b/mobile/android/base/home/BrowserSearch.java index 82c5e5d8a56..8c46a752196 100644 --- a/mobile/android/base/home/BrowserSearch.java +++ b/mobile/android/base/home/BrowserSearch.java @@ -51,7 +51,6 @@ import android.view.View; import android.view.View.OnClickListener; import android.view.ViewGroup; import android.view.ViewStub; -import android.view.WindowManager; import android.view.WindowManager.LayoutParams; import android.view.animation.AccelerateInterpolator; import android.view.animation.Animation; diff --git a/mobile/android/base/home/SearchEngineAdapter.java b/mobile/android/base/home/SearchEngineAdapter.java new file mode 100644 index 00000000000..2226aa070e8 --- /dev/null +++ b/mobile/android/base/home/SearchEngineAdapter.java @@ -0,0 +1,122 @@ +package org.mozilla.gecko.home; + +import android.content.Context; +import android.graphics.drawable.Drawable; +import android.support.v4.content.ContextCompat; +import android.support.v4.graphics.drawable.DrawableCompat; +import android.support.v7.widget.RecyclerView; +import android.view.LayoutInflater; +import android.view.View; +import android.view.ViewGroup; +import android.widget.ImageView; + +import org.mozilla.gecko.R; + +import java.util.Collections; +import java.util.List; + +public class SearchEngineAdapter + extends RecyclerView.Adapter { + + private static final String LOGTAG = SearchEngineAdapter.class.getSimpleName(); + + private static final int VIEW_TYPE_SEARCH_ENGINE = 0; + private static final int VIEW_TYPE_LABEL = 1; + private final Context mContext; + + private int mContainerWidth; + private List mSearchEngines = Collections.emptyList(); + + public void setSearchEngines(List searchEngines) { + mSearchEngines = searchEngines; + notifyDataSetChanged(); + } + + /** + * The container width is used for setting the appropriate calculated amount of width that + * a search engine icon can have. This varies depending on the space available in the + * {@link SearchEngineBar}. The setter exists for this attribute, in creating the view in the + * adapter after said calculation is done when the search bar is created. + * @param iconContainerWidth Width of each search icon. + */ + void setIconContainerWidth(int iconContainerWidth) { + mContainerWidth = iconContainerWidth; + } + + public static class SearchEngineViewHolder extends RecyclerView.ViewHolder { + final private ImageView faviconView; + + public void bindItem(SearchEngine searchEngine) { + faviconView.setImageBitmap(searchEngine.getIcon()); + final String desc = itemView.getResources().getString(R.string.search_bar_item_desc, + searchEngine.getEngineIdentifier()); + itemView.setContentDescription(desc); + } + + public SearchEngineViewHolder(View itemView) { + super(itemView); + faviconView = (ImageView) itemView.findViewById(R.id.search_engine_icon); + } + } + + public SearchEngineAdapter(Context context) { + mContext = context; + } + + @Override + public int getItemViewType(int position) { + return position == 0 ? VIEW_TYPE_LABEL : VIEW_TYPE_SEARCH_ENGINE; + } + + public SearchEngine getItem(int position) { + // We omit the first position which is where the label currently is. + return position == 0 ? null : mSearchEngines.get(position - 1); + } + + @Override + public SearchEngineViewHolder onCreateViewHolder(ViewGroup parent, int viewType) { + switch (viewType) { + case VIEW_TYPE_LABEL: + return new SearchEngineViewHolder(createLabelView(parent)); + case VIEW_TYPE_SEARCH_ENGINE: + return new SearchEngineViewHolder(createSearchEngineView(parent)); + default: + throw new IllegalArgumentException("Unknown view type: " + viewType); + } + } + + @Override + public void onBindViewHolder(SearchEngineViewHolder holder, int position) { + if (position != 0) { + holder.bindItem(getItem(position)); + } + } + + @Override + public int getItemCount() { + return mSearchEngines.size() + 1; + } + + private View createLabelView(ViewGroup parent) { + View view = LayoutInflater.from(mContext) + .inflate(R.layout.search_engine_bar_label, parent, false); + final Drawable icon = DrawableCompat.wrap( + ContextCompat.getDrawable(mContext, R.drawable.search_icon_active).mutate()); + DrawableCompat.setTint(icon, mContext.getResources().getColor(R.color.disabled_grey)); + + final ImageView iconView = (ImageView) view.findViewById(R.id.search_engine_label); + iconView.setImageDrawable(icon); + return view; + } + + private View createSearchEngineView(ViewGroup parent) { + View view = LayoutInflater.from(mContext) + .inflate(R.layout.search_engine_bar_item, parent, false); + + ViewGroup.LayoutParams params = view.getLayoutParams(); + params.width = mContainerWidth; + view.setLayoutParams(params); + + return view; + } +} diff --git a/mobile/android/base/home/SearchEngineBar.java b/mobile/android/base/home/SearchEngineBar.java index 347bb25c4f5..aceb572dc99 100644 --- a/mobile/android/base/home/SearchEngineBar.java +++ b/mobile/android/base/home/SearchEngineBar.java @@ -3,213 +3,144 @@ * 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/. */ - package org.mozilla.gecko.home; +package org.mozilla.gecko.home; import android.content.Context; import android.graphics.Canvas; -import android.graphics.Color; import android.graphics.Paint; -import android.graphics.drawable.Drawable; +import android.support.v7.widget.LinearLayoutManager; +import android.support.v7.widget.RecyclerView; import android.util.AttributeSet; import android.util.DisplayMetrics; import android.util.TypedValue; -import android.view.Gravity; -import android.view.LayoutInflater; import android.view.View; -import android.view.ViewGroup; -import android.widget.AdapterView; -import android.widget.BaseAdapter; -import android.widget.FrameLayout; -import android.widget.ImageView; import org.mozilla.gecko.R; -import org.mozilla.gecko.util.DrawableUtil; -import org.mozilla.gecko.widget.TwoWayView; +import org.mozilla.gecko.mozglue.RobocopTarget; -import java.util.ArrayList; import java.util.List; -public class SearchEngineBar extends TwoWayView - implements AdapterView.OnItemClickListener { - private static final String LOGTAG = "Gecko" + SearchEngineBar.class.getSimpleName(); +public class SearchEngineBar extends RecyclerView + implements RecyclerViewItemClickListener.OnClickListener { + private static final String LOGTAG = SearchEngineBar.class.getSimpleName(); private static final float ICON_CONTAINER_MIN_WIDTH_DP = 72; private static final float LABEL_CONTAINER_WIDTH_DP = 48; private static final float DIVIDER_HEIGHT_DP = 1; public interface OnSearchBarClickListener { - public void onSearchBarClickListener(SearchEngine searchEngine); + void onSearchBarClickListener(SearchEngine searchEngine); } - private final SearchEngineAdapter adapter; - private final Paint dividerPaint; - private final float minIconContainerWidth; - private final float dividerHeight; - private final int labelContainerWidth; + private final SearchEngineAdapter mAdapter; + private final LinearLayoutManager mLayoutManager; + private final Paint mDividerPaint; + private final float mMinIconContainerWidth; + private final float mDividerHeight; + private final int mLabelContainerWidth; - private int iconContainerWidth; - private OnSearchBarClickListener onSearchBarClickListener; + private int mIconContainerWidth; + private OnSearchBarClickListener mOnSearchBarClickListener; public SearchEngineBar(final Context context, final AttributeSet attrs) { super(context, attrs); - dividerPaint = new Paint(); - dividerPaint.setColor(getResources().getColor(R.color.divider_light)); - dividerPaint.setStyle(Paint.Style.FILL_AND_STROKE); + mDividerPaint = new Paint(); + mDividerPaint.setColor(getResources().getColor(R.color.divider_light)); + mDividerPaint.setStyle(Paint.Style.FILL_AND_STROKE); final DisplayMetrics displayMetrics = getResources().getDisplayMetrics(); - minIconContainerWidth = TypedValue.applyDimension(TypedValue.COMPLEX_UNIT_DIP, ICON_CONTAINER_MIN_WIDTH_DP, displayMetrics); - dividerHeight = TypedValue.applyDimension(TypedValue.COMPLEX_UNIT_DIP, DIVIDER_HEIGHT_DP, displayMetrics); - labelContainerWidth = (int) TypedValue.applyDimension(TypedValue.COMPLEX_UNIT_DIP, LABEL_CONTAINER_WIDTH_DP, displayMetrics); + mMinIconContainerWidth = TypedValue.applyDimension( + TypedValue.COMPLEX_UNIT_DIP, ICON_CONTAINER_MIN_WIDTH_DP, displayMetrics); + mDividerHeight = TypedValue.applyDimension( + TypedValue.COMPLEX_UNIT_DIP, DIVIDER_HEIGHT_DP, displayMetrics); + mLabelContainerWidth = Math.round(TypedValue.applyDimension( + TypedValue.COMPLEX_UNIT_DIP, LABEL_CONTAINER_WIDTH_DP, displayMetrics)); - iconContainerWidth = (int) minIconContainerWidth; + mIconContainerWidth = Math.round(mMinIconContainerWidth); - adapter = new SearchEngineAdapter(); - setAdapter(adapter); - setOnItemClickListener(this); + mAdapter = new SearchEngineAdapter(context); + mAdapter.setIconContainerWidth(mIconContainerWidth); + mLayoutManager = new LinearLayoutManager(context); + mLayoutManager.setOrientation(LinearLayoutManager.HORIZONTAL); + + setAdapter(mAdapter); + setLayoutManager(mLayoutManager); + addOnItemTouchListener(new RecyclerViewItemClickListener(context, this, this)); } - @Override - public void onItemClick(final AdapterView parent, final View view, final int position, - final long id) { - if (onSearchBarClickListener == null) { - throw new IllegalStateException( - OnSearchBarClickListener.class.getSimpleName() + " is not initialized"); - } - - if (position == 0) { - // Ignore click on label - return; - } - - final SearchEngine searchEngine = adapter.getItem(position); - onSearchBarClickListener.onSearchBarClickListener(searchEngine); + public void setSearchEngines(List searchEngines) { + mAdapter.setSearchEngines(searchEngines); } - protected void setOnSearchBarClickListener(final OnSearchBarClickListener listener) { - onSearchBarClickListener = listener; - } - - protected void setSearchEngines(final List searchEngines) { - adapter.setSearchEngines(searchEngines); + public void setOnSearchBarClickListener(OnSearchBarClickListener listener) { + mOnSearchBarClickListener = listener; } @Override protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) { super.onMeasure(widthMeasureSpec, heightMeasureSpec); - final int searchEngineCount = adapter.getCount() - 1; + final int searchEngineCount = mAdapter.getItemCount() - 1; if (searchEngineCount > 0) { - final int availableWidth = getMeasuredWidth() - labelContainerWidth; + final int availableWidth = getMeasuredWidth() - mLabelContainerWidth; final double searchEnginesToDisplay; - if (searchEngineCount * minIconContainerWidth <= availableWidth) { + if (searchEngineCount * mMinIconContainerWidth <= availableWidth) { // All search engines fit int: So let's just display all. searchEnginesToDisplay = searchEngineCount; } else { // If only (n) search engines fit into the available space then display (n - 0.5): The last search // engine will be cut-off to show ability to scroll this view - - searchEnginesToDisplay = Math.floor(availableWidth / minIconContainerWidth) - 0.5; + searchEnginesToDisplay = Math.floor(availableWidth / mMinIconContainerWidth) - 0.5; } // Use all available width and spread search engine icons final int availableWidthPerContainer = (int) (availableWidth / searchEnginesToDisplay); - if (availableWidthPerContainer != iconContainerWidth) { - iconContainerWidth = availableWidthPerContainer; - adapter.notifyDataSetChanged(); + if (availableWidthPerContainer != mIconContainerWidth) { + mIconContainerWidth = availableWidthPerContainer; } + mAdapter.setIconContainerWidth(mIconContainerWidth); } } @Override - protected void onDraw(Canvas canvas) { + public void onDraw(Canvas canvas) { super.onDraw(canvas); - canvas.drawRect(0, 0, getWidth(), dividerHeight, dividerPaint); + canvas.drawRect(0, 0, getWidth(), mDividerHeight, mDividerPaint); } - public class SearchEngineAdapter extends BaseAdapter { - private static final int VIEW_TYPE_SEARCH_ENGINE = 0; - private static final int VIEW_TYPE_LABEL = 1; - - List searchEngines = new ArrayList<>(); - - public void setSearchEngines(final List searchEngines) { - this.searchEngines = searchEngines; - notifyDataSetChanged(); + @Override + public void onClick(View view, int position) { + if (mOnSearchBarClickListener == null) { + throw new IllegalStateException( + OnSearchBarClickListener.class.getSimpleName() + " is not initializer." + ); } - @Override - public int getCount() { - // Adding offset for label at position 0 (Bug 1172071) - return searchEngines.size() + 1; + if (position == 0) { + return; } - @Override - public SearchEngine getItem(final int position) { - // Returning null for the label at position 0 (Bug 1172071) - return position == 0 ? null : searchEngines.get(position - 1); - } + final SearchEngine searchEngine = mAdapter.getItem(position); + mOnSearchBarClickListener.onSearchBarClickListener(searchEngine); + } - @Override - public long getItemId(final int position) { - return position; - } + @Override + public void onLongClick(View view, int position) { + // do nothing + } - @Override - public int getItemViewType(int position) { - return position == 0 ? VIEW_TYPE_LABEL : VIEW_TYPE_SEARCH_ENGINE; - } - - @Override - public int getViewTypeCount() { - return 2; - } - - @Override - public View getView(final int position, final View convertView, final ViewGroup parent) { - if (position == 0) { - return getLabelView(convertView, parent); - } else { - return getSearchEngineView(position, convertView, parent); - } - } - - private View getLabelView(View view, final ViewGroup parent) { - if (view == null) { - view = LayoutInflater.from(getContext()).inflate(R.layout.search_engine_bar_label, parent, false); - } - - final Drawable icon = - DrawableUtil.tintDrawable(parent.getContext(), R.drawable.search_icon_active, R.color.disabled_grey); - - final ImageView iconView = (ImageView) view.findViewById(R.id.search_engine_label); - iconView.setImageDrawable(icon); - iconView.setScaleType(ImageView.ScaleType.FIT_XY); - - return view; - } - - private View getSearchEngineView(final int position, View view, final ViewGroup parent) { - if (view == null) { - view = LayoutInflater.from(getContext()).inflate(R.layout.search_engine_bar_item, parent, false); - } - - LayoutParams params = (LayoutParams) view.getLayoutParams(); - params.width = iconContainerWidth; - view.setLayoutParams(params); - - final ImageView faviconView = (ImageView) view.findViewById(R.id.search_engine_icon); - final SearchEngine searchEngine = getItem(position); - faviconView.setImageBitmap(searchEngine.getIcon()); - - final String desc = getResources().getString(R.string.search_bar_item_desc, searchEngine.getEngineIdentifier()); - view.setContentDescription(desc); - - return view; - } + /** + * We manually add the override for getAdapter because we see this method getting stripped + * out during compile time by aggressive proguard rules. + */ + @RobocopTarget + @Override + public SearchEngineAdapter getAdapter() { + return mAdapter; } } diff --git a/mobile/android/base/moz.build b/mobile/android/base/moz.build index e337ea11d73..b468a961da8 100644 --- a/mobile/android/base/moz.build +++ b/mobile/android/base/moz.build @@ -352,6 +352,7 @@ gbjar.sources += [ 'home/RemoteTabsSplitPlaneFragment.java', 'home/RemoteTabsStaticFragment.java', 'home/SearchEngine.java', + 'home/SearchEngineAdapter.java', 'home/SearchEngineBar.java', 'home/SearchEngineRow.java', 'home/SearchLoader.java', diff --git a/mobile/android/base/resources/layout/search_engine_bar_item.xml b/mobile/android/base/resources/layout/search_engine_bar_item.xml index 56d03d156b6..f8c546d9380 100644 --- a/mobile/android/base/resources/layout/search_engine_bar_item.xml +++ b/mobile/android/base/resources/layout/search_engine_bar_item.xml @@ -17,6 +17,7 @@ android:id="@+id/search_engine_icon_container" android:layout_width="72dp" android:layout_height="match_parent" + android:clickable="true" android:background="@color/pressed_about_page_header_grey"> - - - diff --git a/browser/components/loop/content/shared/js/utils.js b/browser/components/loop/content/shared/js/utils.js index 7ce0933a33a..b1867a246c2 100644 --- a/browser/components/loop/content/shared/js/utils.js +++ b/browser/components/loop/content/shared/js/utils.js @@ -740,6 +740,34 @@ var inChrome = typeof Components != "undefined" && "utils" in Components; return obj; } + /** + * Truncate a string if it exceeds the length as defined in `maxLen`, which + * is defined as '72' characters by default. If the string needs trimming, + * it'll be suffixed with the unicode ellipsis char, \u2026. + * + * @param {String} str The string to truncate, if needed. + * @param {Number} maxLen Maximum number of characters that the string is + * allowed to contain. Optional, defaults to 72. + * @return {String} Truncated version of `str`. + */ + function truncate(str, maxLen) { + maxLen = maxLen || 72; + + if (str.length > maxLen) { + var substring = str.substr(0, maxLen); + // XXX Due to the fact that we have two different l10n libraries. + var direction = mozL10n.getDirection ? mozL10n.getDirection() : + mozL10n.language.direction; + if (direction === "rtl") { + return "…" + substring; + } + + return substring + "…"; + } + + return str; + } + this.utils = { CALL_TYPES: CALL_TYPES, FAILURE_DETAILS: FAILURE_DETAILS, @@ -768,6 +796,7 @@ var inChrome = typeof Components != "undefined" && "utils" in Components; strToUint8Array: strToUint8Array, Uint8ArrayToStr: Uint8ArrayToStr, objectDiff: objectDiff, - stripFalsyValues: stripFalsyValues + stripFalsyValues: stripFalsyValues, + truncate: truncate }; }).call(inChrome ? this : loop.shared); diff --git a/browser/components/loop/jar.mn b/browser/components/loop/jar.mn index b72b48c964a..e409302755e 100644 --- a/browser/components/loop/jar.mn +++ b/browser/components/loop/jar.mn @@ -54,7 +54,6 @@ browser.jar: content/browser/loop/shared/img/video-inverse-14x14@2x.png (content/shared/img/video-inverse-14x14@2x.png) content/browser/loop/shared/img/dropdown-inverse.png (content/shared/img/dropdown-inverse.png) content/browser/loop/shared/img/dropdown-inverse@2x.png (content/shared/img/dropdown-inverse@2x.png) - content/browser/loop/shared/img/svg/glyph-settings-16x16.svg (content/shared/img/svg/glyph-settings-16x16.svg) content/browser/loop/shared/img/svg/glyph-account-16x16.svg (content/shared/img/svg/glyph-account-16x16.svg) content/browser/loop/shared/img/svg/glyph-signin-16x16.svg (content/shared/img/svg/glyph-signin-16x16.svg) content/browser/loop/shared/img/svg/glyph-signout-16x16.svg (content/shared/img/svg/glyph-signout-16x16.svg) diff --git a/browser/components/loop/standalone/server.js b/browser/components/loop/standalone/server.js index 6503ac77651..2c57ed0faae 100644 --- a/browser/components/loop/standalone/server.js +++ b/browser/components/loop/standalone/server.js @@ -12,6 +12,8 @@ var express = require("express"); var app = express(); +var path = require("path"); + var port = process.env.PORT || 3000; var feedbackApiUrl = process.env.LOOP_FEEDBACK_API_URL || "https://input.allizom.org/api/v1/feedback"; @@ -62,32 +64,35 @@ app.get("/content/c/config.js", getConfigFile); // /ui - for the ui showcase // /content - for the standalone files. -app.use("/ui", express.static(__dirname + "/../ui")); +app.use("/ui", express.static(path.join(__dirname, "..", "ui"))); +app.use("/ui/loop/", express.static(path.join(__dirname, "..", "content"))); +app.use("/ui/shared/", express.static(path.join(__dirname, "..", "content", + "shared"))); // This exists exclusively for the unit tests. They are served the // whole loop/ directory structure and expect some files in the standalone directory. -app.use("/standalone/content", express.static(__dirname + "/content")); +app.use("/standalone/content", express.static(path.join(__dirname, "content"))); // We load /content this from both /content *and* /../content. The first one // does what we need for running in the github loop-client context, the second one // handles running in the hg repo under mozilla-central and is used so that the shared // files are in the right location. -app.use("/content", express.static(__dirname + "/content")); -app.use("/content", express.static(__dirname + "/../content")); +app.use("/content", express.static(path.join(__dirname, "content"))); +app.use("/content", express.static(path.join(__dirname, "..", "content"))); // These two are based on the above, but handle call urls, that have a /c/ in them. -app.use("/content/c", express.static(__dirname + "/content")); -app.use("/content/c", express.static(__dirname + "/../content")); +app.use("/content/c", express.static(path.join(__dirname, "content"))); +app.use("/content/c", express.static(path.join(__dirname, "..", "content"))); // Two lines for the same reason as /content above. -app.use("/test", express.static(__dirname + "/test")); -app.use("/test", express.static(__dirname + "/../test")); +app.use("/test", express.static(path.join(__dirname, "test"))); +app.use("/test", express.static(path.join(__dirname, "..", "test"))); // As we don't have hashes on the urls, the best way to serve the index files // appears to be to be to closely filter the url and match appropriately. function serveIndex(req, res) { "use strict"; - return res.sendfile(__dirname + "/content/index.html"); + return res.sendfile(path.join(__dirname, "content", "index.html")); } app.get(/^\/content\/[\w\-]+$/, serveIndex); diff --git a/browser/components/loop/test/desktop-local/panel_test.js b/browser/components/loop/test/desktop-local/panel_test.js index f428bd7dfef..96e0729fd59 100644 --- a/browser/components/loop/test/desktop-local/panel_test.js +++ b/browser/components/loop/test/desktop-local/panel_test.js @@ -72,7 +72,8 @@ describe("loop.panel", function() { logOutFromFxA: sandbox.stub(), notifyUITour: sandbox.stub(), openURL: sandbox.stub(), - getSelectedTabMetadata: sandbox.stub() + getSelectedTabMetadata: sandbox.stub(), + userProfile: null }; document.mozL10n.initialize(navigator.mozLoop); @@ -136,9 +137,9 @@ describe("loop.panel", function() { navigator.mozLoop.doNotDisturb = true; }); - it("should toggle the value of mozLoop.doNotDisturb", function() { + it("should toggle mozLoop.doNotDisturb to false", function() { var availableMenuOption = view.getDOMNode() - .querySelector(".dnd-make-available"); + .querySelector(".status-available"); TestUtils.Simulate.click(availableMenuOption); @@ -147,7 +148,7 @@ describe("loop.panel", function() { it("should toggle the dropdown menu", function() { var availableMenuOption = view.getDOMNode() - .querySelector(".dnd-status span"); + .querySelector(".dnd-status span"); TestUtils.Simulate.click(availableMenuOption); @@ -235,8 +236,7 @@ describe("loop.panel", function() { }); }); - describe("AuthLink", function() { - + describe("AccountLink", function() { beforeEach(function() { navigator.mozLoop.calls = { clearCallInProgress: function() {} }; }); @@ -249,13 +249,12 @@ describe("loop.panel", function() { it("should trigger the FxA sign in/up process when clicking the link", function() { - navigator.mozLoop.loggedInToFxA = false; navigator.mozLoop.logInToFxA = sandbox.stub(); var view = createTestPanelView(); TestUtils.Simulate.click( - view.getDOMNode().querySelector(".signin-link a")); + view.getDOMNode().querySelector(".signin-link > a")); sinon.assert.calledOnce(navigator.mozLoop.logInToFxA); }); @@ -268,7 +267,7 @@ describe("loop.panel", function() { var view = createTestPanelView(); TestUtils.Simulate.click( - view.getDOMNode().querySelector(".signin-link a")); + view.getDOMNode().querySelector(".signin-link > a")); sinon.assert.calledOnce(fakeWindow.close); }); @@ -277,9 +276,50 @@ describe("loop.panel", function() { function() { navigator.mozLoop.fxAEnabled = false; var view = TestUtils.renderIntoDocument( - React.createElement(loop.panel.AuthLink)); + React.createElement(loop.panel.AccountLink, { + fxAEnabled: false, + userProfile: null + })); expect(view.getDOMNode()).to.be.null; }); + + it("should add ellipsis to text over 24chars", function() { + navigator.mozLoop.userProfile = { + email: "reallyreallylongtext@example.com" + }; + var view = createTestPanelView(); + var node = view.getDOMNode().querySelector(".user-identity"); + + expect(node.textContent).to.eql("reallyreallylongtext@exa…"); + }); + + it("should throw an error when user profile is different from {} or null", + function() { + var warnstub = sandbox.stub(console, "warn"); + var view = TestUtils.renderIntoDocument(React.createElement( + loop.panel.AccountLink, { + fxAEnabled: false, + userProfile: [] + } + )); + + sinon.assert.calledOnce(warnstub); + sinon.assert.calledWithExactly(warnstub, "Warning: Required prop `userProfile` was not correctly specified in `AccountLink`."); + }); + + it("should throw an error when user profile is different from {} or null", + function() { + var warnstub = sandbox.stub(console, "warn"); + var view = TestUtils.renderIntoDocument(React.createElement( + loop.panel.AccountLink, { + fxAEnabled: false, + userProfile: function() {} + } + )); + + sinon.assert.calledOnce(warnstub); + sinon.assert.calledWithExactly(warnstub, "Warning: Required prop `userProfile` was not correctly specified in `AccountLink`."); + }); }); describe("SettingsDropdown", function() { @@ -300,17 +340,39 @@ describe("loop.panel", function() { navigator.mozLoop.fxAEnabled = true; }); - it("should show a signin entry when user is not authenticated", - function() { - navigator.mozLoop.loggedInToFxA = false; + describe("UserLoggedOut", function() { + beforeEach(function() { + fakeMozLoop.userProfile = null; + }); + it("should show a signin entry when user is not authenticated", + function() { + var view = mountTestComponent(); + + expect(view.getDOMNode().querySelectorAll(".icon-signout")) + .to.have.length.of(0); + expect(view.getDOMNode().querySelectorAll(".icon-signin")) + .to.have.length.of(1); + }); + + it("should hide any account entry when user is not authenticated", + function() { + var view = mountTestComponent(); + + expect(view.getDOMNode().querySelectorAll(".icon-account")) + .to.have.length.of(0); + }); + + it("should sign in the user on click when unauthenticated", function() { + navigator.mozLoop.loggedInToFxA = false; var view = mountTestComponent(); - expect(view.getDOMNode().querySelectorAll(".icon-signout")) - .to.have.length.of(0); - expect(view.getDOMNode().querySelectorAll(".icon-signin")) - .to.have.length.of(1); + TestUtils.Simulate.click(view.getDOMNode() + .querySelector(".icon-signin")); + + sinon.assert.calledOnce(navigator.mozLoop.logInToFxA); }); + }); it("should show a signout entry when user is authenticated", function() { navigator.mozLoop.userProfile = {email: "test@example.com"}; @@ -332,43 +394,24 @@ describe("loop.panel", function() { .to.have.length.of(1); }); - it("should open the FxA settings when the account entry is clicked", function() { - navigator.mozLoop.userProfile = {email: "test@example.com"}; + it("should open the FxA settings when the account entry is clicked", + function() { + navigator.mozLoop.userProfile = {email: "test@example.com"}; - var view = mountTestComponent(); + var view = mountTestComponent(); - TestUtils.Simulate.click( - view.getDOMNode().querySelector(".icon-account")); + TestUtils.Simulate.click(view.getDOMNode() + .querySelector(".icon-account")); - sinon.assert.calledOnce(navigator.mozLoop.openFxASettings); - }); - - it("should hide any account entry when user is not authenticated", - function() { - navigator.mozLoop.loggedInToFxA = false; - - var view = mountTestComponent(); - - expect(view.getDOMNode().querySelectorAll(".icon-account")) - .to.have.length.of(0); - }); - - it("should sign in the user on click when unauthenticated", function() { - navigator.mozLoop.loggedInToFxA = false; - var view = mountTestComponent(); - - TestUtils.Simulate.click( - view.getDOMNode().querySelector(".icon-signin")); - - sinon.assert.calledOnce(navigator.mozLoop.logInToFxA); - }); + sinon.assert.calledOnce(navigator.mozLoop.openFxASettings); + }); it("should sign out the user on click when authenticated", function() { navigator.mozLoop.userProfile = {email: "test@example.com"}; var view = mountTestComponent(); - TestUtils.Simulate.click( - view.getDOMNode().querySelector(".icon-signout")); + TestUtils.Simulate.click(view.getDOMNode() + .querySelector(".icon-signout")); sinon.assert.calledOnce(navigator.mozLoop.logOutFromFxA); }); @@ -724,7 +767,8 @@ describe("loop.panel", function() { store: roomStore, dispatcher: dispatcher, userDisplayName: fakeEmail, - mozLoop: fakeMozLoop + mozLoop: fakeMozLoop, + userProfile: null })); } diff --git a/browser/components/loop/test/mochitest/browser_fxa_login.js b/browser/components/loop/test/mochitest/browser_fxa_login.js index e3a6b65047e..d34c3de492a 100644 --- a/browser/components/loop/test/mochitest/browser_fxa_login.js +++ b/browser/components/loop/test/mochitest/browser_fxa_login.js @@ -289,8 +289,10 @@ add_task(function* basicAuthorizationAndRegistration() { yield loadLoopPanel({stayOnline: true}); yield statusChangedPromise; let loopDoc = document.getElementById("loop-panel-iframe").contentDocument; - let visibleEmail = loopDoc.getElementsByClassName("user-identity")[0]; - is(visibleEmail.textContent, "Guest", "Guest should be displayed on the panel when not logged in"); + let accountLogin = loopDoc.getElementsByClassName("signin-link")[0]; + let visibleEmail = loopDoc.getElementsByClassName("user-identity"); + is(visibleEmail.length, 0, "No email should be displayed when logged out"); + is(accountLogin.textContent, "Sign In or Sign Up", "Login/Signup links when logged out"); is(MozLoopService.userProfile, null, "profile should be null before log-in"); let loopButton = document.getElementById("loop-button"); is(loopButton.getAttribute("state"), "", "state of loop button should be empty when not logged in"); @@ -303,6 +305,8 @@ add_task(function* basicAuthorizationAndRegistration() { is(tokenData.scope, "profile", "Check scope"); is(tokenData.token_type, "bearer", "Check token_type"); + visibleEmail = loopDoc.getElementsByClassName("user-identity")[0]; + is(MozLoopService.userProfile.email, "test@example.com", "email should exist in the profile data"); is(MozLoopService.userProfile.uid, "1234abcd", "uid should exist in the profile data"); is(visibleEmail.textContent, "test@example.com", "the email should be correct on the panel"); @@ -328,7 +332,7 @@ add_task(function* basicAuthorizationAndRegistration() { registrationResponse = yield promiseOAuthGetRegistration(BASE_URL); is(registrationResponse.response, null, "Check registration was deleted on the server"); - is(visibleEmail.textContent, "Guest", "Guest should be displayed on the panel again after logout"); + is(accountLogin.textContent, "Sign In or Sign Up", "Login/Signup links when logged out"); is(MozLoopService.userProfile, null, "userProfile should be null after logout"); }); diff --git a/browser/components/loop/test/shared/utils_test.js b/browser/components/loop/test/shared/utils_test.js index 9797cac49a7..b6888ccf774 100644 --- a/browser/components/loop/test/shared/utils_test.js +++ b/browser/components/loop/test/shared/utils_test.js @@ -665,4 +665,45 @@ describe("loop.shared.utils", function() { expect(obj).to.eql({ prop1: "null", prop3: true }); }); }); + + describe("#truncate", function() { + describe("ltr support", function() { + it("should default to 72 chars", function() { + var output = sharedUtils.truncate(new Array(75).join()); + + expect(output.length).to.eql(73); // 72 + … + }); + + it("should take a max size argument", function() { + var output = sharedUtils.truncate(new Array(73).join(), 20); + + expect(output.length).to.eql(21); // 20 + … + }); + }); + + describe("rtl support", function() { + var directionStub; + + beforeEach(function() { + // XXX should use sandbox + // https://github.com/cjohansen/Sinon.JS/issues/781 + directionStub = sinon.stub(navigator.mozL10n.language, "direction", { + get: function() { + return "rtl"; + } + }); + }); + + afterEach(function() { + directionStub.restore(); + }); + + it("should support RTL", function() { + var output = sharedUtils.truncate(new Array(73).join(), 20); + + expect(output.length).to.eql(21); // 20 + … + expect(output.substr(0, 1)).to.eql("…"); + }); + }); + }); }); diff --git a/browser/components/loop/ui/fake-l10n.js b/browser/components/loop/ui/fake-l10n.js index ebe347f46c0..096e0df00b3 100644 --- a/browser/components/loop/ui/fake-l10n.js +++ b/browser/components/loop/ui/fake-l10n.js @@ -14,7 +14,13 @@ navigator.mozL10n = document.mozL10n = { initialize: function(){}, - getDirection: function(){}, + getDirection: function(){ + if (document.location.search === "?rtl=1") { + return "rtl"; + } + + return "ltr"; + }, get: function(stringId, vars) { diff --git a/browser/components/loop/ui/fake-mozLoop.js b/browser/components/loop/ui/fake-mozLoop.js index f651ccbf247..fee97d346be 100644 --- a/browser/components/loop/ui/fake-mozLoop.js +++ b/browser/components/loop/ui/fake-mozLoop.js @@ -164,6 +164,7 @@ var fakeContacts = [{ }, fxAEnabled: true, startAlerting: function() {}, - stopAlerting: function() {} + stopAlerting: function() {}, + userProfile: null }; })(); diff --git a/browser/components/loop/ui/ui-showcase.css b/browser/components/loop/ui/ui-showcase.css index d7cd3507782..3a47951f3ab 100644 --- a/browser/components/loop/ui/ui-showcase.css +++ b/browser/components/loop/ui/ui-showcase.css @@ -165,3 +165,9 @@ body { .standalone.text-chat-example .text-chat-view { height: 400px; } + +/* Force dropdown menus to display. */ +.force-menu-show * { + display: inline-block !important; +} + diff --git a/browser/components/loop/ui/ui-showcase.js b/browser/components/loop/ui/ui-showcase.js index ffba63df3c4..ca9d270c71b 100644 --- a/browser/components/loop/ui/ui-showcase.js +++ b/browser/components/loop/ui/ui-showcase.js @@ -15,6 +15,7 @@ // 1. Desktop components // 1.1 Panel + var AvailabilityDropdown = loop.panel.AvailabilityDropdown; var PanelView = loop.panel.PanelView; var SignInRequestView = loop.panel.SignInRequestView; // 1.2. Conversation Window @@ -438,7 +439,17 @@ // Local mocks - var mockMozLoopRooms = _.extend({}, navigator.mozLoop); + var mockMozLoopLoggedIn = _.cloneDeep(navigator.mozLoop); + mockMozLoopLoggedIn.userProfile = { + email: "text@example.com", + uid: "0354b278a381d3cb408bb46ffc01266" + }; + + var mockMozLoopLoggedInLongEmail = _.cloneDeep(navigator.mozLoop); + mockMozLoopLoggedInLongEmail.userProfile = { + email: "reallyreallylongtext@example.com", + uid: "0354b278a381d3cb408bb46ffc01266" + }; var mockContact = { name: ["Mr Smith"], @@ -492,7 +503,8 @@ "10x10": ["close", "close-active", "close-disabled", "dropdown", "dropdown-white", "dropdown-active", "dropdown-disabled", "edit", "edit-active", "edit-disabled", "edit-white", "expand", "expand-active", - "expand-disabled", "minimize", "minimize-active", "minimize-disabled" + "expand-disabled", "minimize", "minimize-active", "minimize-disabled", + "settings-cog" ], "14x14": ["audio", "audio-active", "audio-disabled", "facemute", "facemute-active", "facemute-disabled", "hangup", "hangup-active", @@ -509,7 +521,8 @@ "precall", "precall-hover", "precall-active", "screen-white", "screenmute-white", "settings", "settings-hover", "settings-active", "share-darkgrey", "tag", "tag-hover", "tag-active", "trash", "unblock", "unblock-hover", "unblock-active", - "video", "video-hover", "video-active", "tour" + "video", "video-hover", "video-active", "tour", "status-available", + "status-unavailable" ] }, @@ -580,6 +593,7 @@ React.PropTypes.element, React.PropTypes.arrayOf(React.PropTypes.element) ]).isRequired, + cssClass: React.PropTypes.string, dashed: React.PropTypes.bool, style: React.PropTypes.object, summary: React.PropTypes.string.isRequired @@ -591,8 +605,14 @@ render: function() { var cx = React.addons.classSet; + var extraCSSClass = { + "example": true + }; + if (this.props.cssClass) { + extraCSSClass[this.props.cssClass] = true; + } return ( - React.createElement("div", {className: "example"}, + React.createElement("div", {className: cx(extraCSSClass)}, React.createElement("h3", {id: this.makeId()}, this.props.summary, React.createElement("a", {href: this.makeId("#")}, " ¶") @@ -693,25 +713,31 @@ React.createElement("strong", null, "Note:"), " 332px wide." ), React.createElement(Example, {dashed: true, style: {width: "332px"}, summary: "Re-sign-in view"}, - React.createElement(SignInRequestView, {mozLoop: mockMozLoopRooms}) + React.createElement(SignInRequestView, {mozLoop: mockMozLoopLoggedIn}) ), React.createElement(Example, {dashed: true, style: {width: "332px"}, summary: "Room list tab"}, React.createElement(PanelView, {client: mockClient, dispatcher: dispatcher, - mozLoop: mockMozLoopRooms, + mozLoop: mockMozLoopLoggedIn, notifications: notifications, roomStore: roomStore, - selectedTab: "rooms", - userProfile: {email: "test@example.com"}}) + selectedTab: "rooms"}) ), React.createElement(Example, {dashed: true, style: {width: "332px"}, summary: "Contact list tab"}, React.createElement(PanelView, {client: mockClient, dispatcher: dispatcher, - mozLoop: mockMozLoopRooms, + mozLoop: mockMozLoopLoggedIn, notifications: notifications, roomStore: roomStore, - selectedTab: "contacts", - userProfile: {email: "test@example.com"}}) + selectedTab: "contacts"}) + ), + React.createElement(Example, {dashed: true, style: {width: "332px"}, summary: "Contact list tab long email"}, + React.createElement(PanelView, {client: mockClient, + dispatcher: dispatcher, + mozLoop: mockMozLoopLoggedInLongEmail, + notifications: notifications, + roomStore: roomStore, + selectedTab: "contacts"}) ), React.createElement(Example, {dashed: true, style: {width: "332px"}, summary: "Error Notification"}, React.createElement(PanelView, {client: mockClient, @@ -723,26 +749,38 @@ React.createElement(Example, {dashed: true, style: {width: "332px"}, summary: "Error Notification - authenticated"}, React.createElement(PanelView, {client: mockClient, dispatcher: dispatcher, - mozLoop: navigator.mozLoop, + mozLoop: mockMozLoopLoggedIn, notifications: errNotifications, - roomStore: roomStore, - userProfile: {email: "test@example.com"}}) + roomStore: roomStore}) ), React.createElement(Example, {dashed: true, style: {width: "332px"}, summary: "Contact import success"}, React.createElement(PanelView, {dispatcher: dispatcher, - mozLoop: mockMozLoopRooms, + mozLoop: mockMozLoopLoggedIn, notifications: new loop.shared.models.NotificationCollection([{level: "success", message: "Import success"}]), roomStore: roomStore, - selectedTab: "contacts", - userProfile: {email: "test@example.com"}}) + selectedTab: "contacts"}) ), React.createElement(Example, {dashed: true, style: {width: "332px"}, summary: "Contact import error"}, React.createElement(PanelView, {dispatcher: dispatcher, - mozLoop: mockMozLoopRooms, + mozLoop: mockMozLoopLoggedIn, notifications: new loop.shared.models.NotificationCollection([{level: "error", message: "Import error"}]), roomStore: roomStore, - selectedTab: "contacts", - userProfile: {email: "test@example.com"}}) + selectedTab: "contacts"}) + ) + ), + + React.createElement(Section, {name: "Availability Dropdown"}, + React.createElement("p", {className: "note"}, + React.createElement("strong", null, "Note:"), " 332px wide." + ), + React.createElement(Example, {dashed: true, style: {width: "332px", height: "200px"}, + summary: "AvailabilityDropdown"}, + React.createElement(AvailabilityDropdown, null) + ), + React.createElement(Example, {cssClass: "force-menu-show", dashed: true, + style: {width: "332px", height: "200px"}, + summary: "AvailabilityDropdown Expanded"}, + React.createElement(AvailabilityDropdown, null) ) ), @@ -753,7 +791,7 @@ React.createElement(AcceptCallView, {callType: CALL_TYPES.AUDIO_VIDEO, callerId: "Mr Smith", dispatcher: dispatcher, - mozLoop: mockMozLoopRooms}) + mozLoop: mockMozLoopLoggedIn}) ) ), @@ -763,7 +801,7 @@ React.createElement(AcceptCallView, {callType: CALL_TYPES.AUDIO_ONLY, callerId: "Mr Smith", dispatcher: dispatcher, - mozLoop: mockMozLoopRooms}) + mozLoop: mockMozLoopLoggedIn}) ) ) ), @@ -775,7 +813,7 @@ React.createElement(AcceptCallView, {callType: CALL_TYPES.AUDIO_VIDEO, callerId: "Mr Smith", dispatcher: dispatcher, - mozLoop: mockMozLoopRooms, + mozLoop: mockMozLoopLoggedIn, showMenu: true}) ) ) diff --git a/browser/components/loop/ui/ui-showcase.jsx b/browser/components/loop/ui/ui-showcase.jsx index 845d7946fc9..98041b945ab 100644 --- a/browser/components/loop/ui/ui-showcase.jsx +++ b/browser/components/loop/ui/ui-showcase.jsx @@ -15,6 +15,7 @@ // 1. Desktop components // 1.1 Panel + var AvailabilityDropdown = loop.panel.AvailabilityDropdown; var PanelView = loop.panel.PanelView; var SignInRequestView = loop.panel.SignInRequestView; // 1.2. Conversation Window @@ -438,7 +439,17 @@ // Local mocks - var mockMozLoopRooms = _.extend({}, navigator.mozLoop); + var mockMozLoopLoggedIn = _.cloneDeep(navigator.mozLoop); + mockMozLoopLoggedIn.userProfile = { + email: "text@example.com", + uid: "0354b278a381d3cb408bb46ffc01266" + }; + + var mockMozLoopLoggedInLongEmail = _.cloneDeep(navigator.mozLoop); + mockMozLoopLoggedInLongEmail.userProfile = { + email: "reallyreallylongtext@example.com", + uid: "0354b278a381d3cb408bb46ffc01266" + }; var mockContact = { name: ["Mr Smith"], @@ -492,7 +503,8 @@ "10x10": ["close", "close-active", "close-disabled", "dropdown", "dropdown-white", "dropdown-active", "dropdown-disabled", "edit", "edit-active", "edit-disabled", "edit-white", "expand", "expand-active", - "expand-disabled", "minimize", "minimize-active", "minimize-disabled" + "expand-disabled", "minimize", "minimize-active", "minimize-disabled", + "settings-cog" ], "14x14": ["audio", "audio-active", "audio-disabled", "facemute", "facemute-active", "facemute-disabled", "hangup", "hangup-active", @@ -509,7 +521,8 @@ "precall", "precall-hover", "precall-active", "screen-white", "screenmute-white", "settings", "settings-hover", "settings-active", "share-darkgrey", "tag", "tag-hover", "tag-active", "trash", "unblock", "unblock-hover", "unblock-active", - "video", "video-hover", "video-active", "tour" + "video", "video-hover", "video-active", "tour", "status-available", + "status-unavailable" ] }, @@ -580,6 +593,7 @@ React.PropTypes.element, React.PropTypes.arrayOf(React.PropTypes.element) ]).isRequired, + cssClass: React.PropTypes.string, dashed: React.PropTypes.bool, style: React.PropTypes.object, summary: React.PropTypes.string.isRequired @@ -591,8 +605,14 @@ render: function() { var cx = React.addons.classSet; + var extraCSSClass = { + "example": true + }; + if (this.props.cssClass) { + extraCSSClass[this.props.cssClass] = true; + } return ( -
+

{this.props.summary}  Â¶ @@ -693,25 +713,31 @@ Note: 332px wide.

- + + selectedTab="rooms" /> + selectedTab="contacts" /> + + + + roomStore={roomStore} /> + selectedTab="contacts" /> + selectedTab="contacts" /> + + + +
+

+ Note: 332px wide. +

+ + + + +
@@ -753,7 +791,7 @@ + mozLoop={mockMozLoopLoggedIn} />

@@ -763,7 +801,7 @@ + mozLoop={mockMozLoopLoggedIn} />
@@ -775,7 +813,7 @@ From fec865efcf6ecf567ade867bb63f3cdb7713c4b5 Mon Sep 17 00:00:00 2001 From: Gabriel Luong Date: Tue, 4 Aug 2015 15:03:02 -0700 Subject: [PATCH 17/28] Bug 1190047 - [Rule View] Ensure changes to swatches are reverted to the original value on escape r=bgrins --- browser/devtools/shared/widgets/Tooltip.js | 5 + browser/devtools/styleinspector/rule-view.js | 50 ++++++-- ...wser_ruleview_colorpicker-revert-on-ESC.js | 111 +++++++++++++---- ...wser_ruleview_cubicbezier-revert-on-ESC.js | 116 ++++++++++++++---- ...ser_ruleview_filtereditor-revert-on-ESC.js | 96 ++++++++++++--- 5 files changed, 305 insertions(+), 73 deletions(-) diff --git a/browser/devtools/shared/widgets/Tooltip.js b/browser/devtools/shared/widgets/Tooltip.js index e19528e5d27..4ddb3f441a5 100644 --- a/browser/devtools/shared/widgets/Tooltip.js +++ b/browser/devtools/shared/widgets/Tooltip.js @@ -1025,6 +1025,7 @@ SwatchBasedEditorTooltip.prototype = { * @param {object} callbacks * Callbacks that will be executed when the editor wants to preview a * value change, or revert a change, or commit a change. + * - onShow: will be called when one of the swatch tooltip is shown * - onPreview: will be called when one of the sub-classes calls * preview * - onRevert: will be called when the user ESCapes out of the tooltip @@ -1032,6 +1033,9 @@ SwatchBasedEditorTooltip.prototype = { * outside the tooltip. */ addSwatch: function(swatchEl, callbacks={}) { + if (!callbacks.onShow) { + callbacks.onShow = function() {}; + } if (!callbacks.onPreview) { callbacks.onPreview = function() {}; } @@ -1069,6 +1073,7 @@ SwatchBasedEditorTooltip.prototype = { if (swatch) { this.activeSwatch = event.target; this.show(); + swatch.callbacks.onShow(); event.stopPropagation(); } }, diff --git a/browser/devtools/styleinspector/rule-view.js b/browser/devtools/styleinspector/rule-view.js index 0440747d668..6624246c3ce 100644 --- a/browser/devtools/styleinspector/rule-view.js +++ b/browser/devtools/styleinspector/rule-view.js @@ -2849,6 +2849,9 @@ function TextPropertyEditor(aRuleEditor, aProperty) { this._onStartEditing = this._onStartEditing.bind(this); this._onNameDone = this._onNameDone.bind(this); this._onValueDone = this._onValueDone.bind(this); + this._onSwatchCommit = this._onSwatchCommit.bind(this); + this._onSwatchPreview = this._onSwatchPreview.bind(this); + this._onSwatchRevert = this._onSwatchRevert.bind(this); this._onValidate = throttle(this._previewValue, 10, this); this.update = this.update.bind(this); @@ -3075,7 +3078,7 @@ TextPropertyEditor.prototype = { this.warning.hidden = this.editing || this.isValid(); - if ((this.prop.overridden || !this.prop.enabled) && !this.editing) { + if (this.prop.overridden || !this.prop.enabled) { this.element.classList.add("ruleview-overridden"); } else { this.element.classList.remove("ruleview-overridden"); @@ -3130,9 +3133,10 @@ TextPropertyEditor.prototype = { // Adding this swatch to the list of swatches our colorpicker // knows about this.ruleView.tooltips.colorPicker.addSwatch(span, { - onPreview: () => this._previewValue(this.valueSpan.textContent), - onCommit: () => this._onValueDone(this.valueSpan.textContent, true), - onRevert: () => this._onValueDone(undefined, false) + onShow: this._onStartEditing, + onPreview: this._onSwatchPreview, + onCommit: this._onSwatchCommit, + onRevert: this._onSwatchRevert }); } } @@ -3145,9 +3149,10 @@ TextPropertyEditor.prototype = { // Adding this swatch to the list of swatches our colorpicker // knows about this.ruleView.tooltips.cubicBezier.addSwatch(span, { - onPreview: () => this._previewValue(this.valueSpan.textContent), - onCommit: () => this._onValueDone(this.valueSpan.textContent, true), - onRevert: () => this._onValueDone(undefined, false) + onShow: this._onStartEditing, + onPreview: this._onSwatchPreview, + onCommit: this._onSwatchCommit, + onRevert: this._onSwatchRevert }); } } @@ -3159,9 +3164,10 @@ TextPropertyEditor.prototype = { parserOptions.filterSwatch = true; this.ruleView.tooltips.filterEditor.addSwatch(span, { - onPreview: () => this._previewValue(this.valueSpan.textContent), - onCommit: () => this._onValueDone(this.valueSpan.textContent, true), - onRevert: () => this._onValueDone(undefined, false) + onShow: this._onStartEditing, + onPreview: this._onSwatchPreview, + onCommit: this._onSwatchCommit, + onRevert: this._onSwatchRevert }, outputParser, parserOptions); } } @@ -3422,6 +3428,30 @@ TextPropertyEditor.prototype = { } }, + /** + * Called when the swatch editor wants to commit a value change. + */ + _onSwatchCommit: function() { + this._onValueDone(this.valueSpan.textContent, true); + this.update(); + }, + + /** + * Called when the swatch editor wants to preview a value change. + */ + _onSwatchPreview: function() { + this._previewValue(this.valueSpan.textContent); + }, + + /** + * Called when the swatch editor closes from an ESC. Revert to the original + * value of this property before editing. + */ + _onSwatchRevert: function() { + this.rule.setPropertyEnabled(this.prop, this.prop.enabled); + this.update(); + }, + /** * Parse a value string and break it into pieces, starting with the * first value, and into an array of additional properties (if any). diff --git a/browser/devtools/styleinspector/test/browser_ruleview_colorpicker-revert-on-ESC.js b/browser/devtools/styleinspector/test/browser_ruleview_colorpicker-revert-on-ESC.js index 3a16a4aed8b..7ac2b0156e3 100644 --- a/browser/devtools/styleinspector/test/browser_ruleview_colorpicker-revert-on-ESC.js +++ b/browser/devtools/styleinspector/test/browser_ruleview_colorpicker-revert-on-ESC.js @@ -6,33 +6,32 @@ // Test that a color change in the color picker is reverted when ESC is pressed -const PAGE_CONTENT = [ - '', - 'Testing the color picker tooltip!' +let TEST_URI = [ + "", ].join("\n"); add_task(function*() { - yield addTab("data:text/html;charset=utf-8,rule view color picker tooltip test"); - content.document.body.innerHTML = PAGE_CONTENT; - let {toolbox, inspector, view} = yield openRuleView(); - - let swatch = getRuleViewProperty(view, "body", "background-color").valueSpan - .querySelector(".ruleview-colorswatch"); - yield testPressingEscapeRevertsChanges(swatch, view); + yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI)); + let {view} = yield openRuleView(); + yield testPressingEscapeRevertsChanges(view); + yield testPressingEscapeRevertsChangesAndDisables(view); }); -function* testPressingEscapeRevertsChanges(swatch, ruleView) { - let cPicker = ruleView.tooltips.colorPicker; +function* testPressingEscapeRevertsChanges(view) { + let ruleEditor = getRuleViewRuleEditor(view, 1); + let propEditor = ruleEditor.rule.textProps[0].editor; + let swatch = propEditor.valueSpan.querySelector(".ruleview-colorswatch"); + let cPicker = view.tooltips.colorPicker; let onShown = cPicker.tooltip.once("shown"); swatch.click(); yield onShown; - yield simulateColorPickerChange(ruleView, cPicker, [0, 0, 0, 1], { + yield simulateColorPickerChange(view, cPicker, [0, 0, 0, 1], { element: content.document.body, name: "backgroundColor", value: "rgb(0, 0, 0)" @@ -40,17 +39,83 @@ function* testPressingEscapeRevertsChanges(swatch, ruleView) { is(swatch.style.backgroundColor, "rgb(0, 0, 0)", "The color swatch's background was updated"); - is(getRuleViewProperty(ruleView, "body", "background-color").valueSpan.textContent, - "#000", "The text of the background-color css property was updated"); + is(propEditor.valueSpan.textContent, "#000", + "The text of the background-color css property was updated"); let spectrum = yield cPicker.spectrum; - // ESC out of the color picker + info("Pressing ESCAPE to close the tooltip"); let onHidden = cPicker.tooltip.once("hidden"); EventUtils.sendKey("ESCAPE", spectrum.element.ownerDocument.defaultView); yield onHidden; + yield ruleEditor.rule._applyingModifications; - yield waitForSuccess(() => { - return content.getComputedStyle(content.document.body).backgroundColor === "rgb(237, 237, 237)"; - }, "The element's background-color was reverted"); + yield waitForComputedStyleProperty("body", null, "background-color", + "rgb(237, 237, 237)"); + is(propEditor.valueSpan.textContent, "#EDEDED", + "Got expected property value."); +} + +function* testPressingEscapeRevertsChangesAndDisables(view) { + let ruleEditor = getRuleViewRuleEditor(view, 1); + let propEditor = ruleEditor.rule.textProps[0].editor; + let swatch = propEditor.valueSpan.querySelector(".ruleview-colorswatch"); + let cPicker = view.tooltips.colorPicker; + + info("Disabling background-color property"); + propEditor.enable.click(); + yield ruleEditor.rule._applyingModifications; + + ok(propEditor.element.classList.contains("ruleview-overridden"), + "property is overridden."); + is(propEditor.enable.style.visibility, "visible", + "property enable checkbox is visible."); + ok(!propEditor.enable.getAttribute("checked"), + "property enable checkbox is not checked."); + ok(!propEditor.prop.enabled, + "background-color property is disabled."); + let newValue = yield getRulePropertyValue("background-color"); + is(newValue, "", "background-color should have been unset."); + + let onShown = cPicker.tooltip.once("shown"); + swatch.click(); + yield onShown; + + ok(!propEditor.element.classList.contains("ruleview-overridden"), + "property overridden is not displayed."); + is(propEditor.enable.style.visibility, "hidden", + "property enable checkbox is hidden."); + + let spectrum = yield cPicker.spectrum; + info("Simulating a color picker change in the widget"); + spectrum.rgb = [0, 0, 0, 1]; + yield ruleEditor.rule._applyingModifications; + + info("Pressing ESCAPE to close the tooltip"); + let onHidden = cPicker.tooltip.once("hidden"); + EventUtils.sendKey("ESCAPE", spectrum.element.ownerDocument.defaultView); + yield onHidden; + yield ruleEditor.rule._applyingModifications; + + ok(propEditor.element.classList.contains("ruleview-overridden"), + "property is overridden."); + is(propEditor.enable.style.visibility, "visible", + "property enable checkbox is visible."); + ok(!propEditor.enable.getAttribute("checked"), + "property enable checkbox is not checked."); + ok(!propEditor.prop.enabled, + "background-color property is disabled."); + newValue = yield getRulePropertyValue("background-color"); + is(newValue, "", "background-color should have been unset."); + is(propEditor.valueSpan.textContent, "#EDEDED", + "Got expected property value."); +} + +function* getRulePropertyValue(name) { + let propValue = yield executeInContent("Test:GetRulePropertyValue", { + styleSheetIndex: 0, + ruleIndex: 0, + name: name + }); + return propValue; } diff --git a/browser/devtools/styleinspector/test/browser_ruleview_cubicbezier-revert-on-ESC.js b/browser/devtools/styleinspector/test/browser_ruleview_cubicbezier-revert-on-ESC.js index 998ab2021c7..d0bbac9db51 100644 --- a/browser/devtools/styleinspector/test/browser_ruleview_cubicbezier-revert-on-ESC.js +++ b/browser/devtools/styleinspector/test/browser_ruleview_cubicbezier-revert-on-ESC.js @@ -4,30 +4,29 @@ "use strict"; -// Test that changes made to the cubic-bezier timing-function in the cubic-bezier -// tooltip are reverted when ESC is pressed +// Test that changes made to the cubic-bezier timing-function in the +// cubic-bezier tooltip are reverted when ESC is pressed -const PAGE_CONTENT = [ - '', +let TEST_URI = [ + "", ].join("\n"); add_task(function*() { - yield addTab("data:text/html;charset=utf-8,rule view cubic-bezier tooltip test"); - content.document.body.innerHTML = PAGE_CONTENT; - let {toolbox, inspector, view} = yield openRuleView(); - - info("Getting the bezier swatch element"); - let swatch = getRuleViewProperty(view, "body", "animation-timing-function").valueSpan - .querySelector(".ruleview-bezierswatch"); - yield testPressingEscapeRevertsChanges(swatch, view); + yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI)); + let {view} = yield openRuleView(); + yield testPressingEscapeRevertsChanges(view); + yield testPressingEscapeRevertsChangesAndDisables(view); }); -function* testPressingEscapeRevertsChanges(swatch, ruleView) { - let bezierTooltip = ruleView.tooltips.cubicBezier; +function* testPressingEscapeRevertsChanges(view) { + let ruleEditor = getRuleViewRuleEditor(view, 1); + let propEditor = ruleEditor.rule.textProps[0].editor; + let swatch = propEditor.valueSpan.querySelector(".ruleview-bezierswatch"); + let bezierTooltip = view.tooltips.cubicBezier; let onShown = bezierTooltip.tooltip.once("shown"); swatch.click(); @@ -36,18 +35,85 @@ function* testPressingEscapeRevertsChanges(swatch, ruleView) { let widget = yield bezierTooltip.widget; info("Simulating a change of curve in the widget"); widget.coordinates = [0.1, 2, 0.9, -1]; - let expected = "cubic-bezier(0.1, 2, 0.9, -1)"; + yield ruleEditor.rule._applyingModifications; - yield waitForSuccess(() => { - return content.getComputedStyle(content.document.body).animationTimingFunction === expected; - }, "Waiting for the change to be previewed on the element"); + yield waitForComputedStyleProperty("body", null, "animation-timing-function", + "cubic-bezier(0.1, 2, 0.9, -1)"); + is(propEditor.valueSpan.textContent, "cubic-bezier(.1,2,.9,-1)", + "Got expected property value."); info("Pressing ESCAPE to close the tooltip"); let onHidden = bezierTooltip.tooltip.once("hidden"); EventUtils.sendKey("ESCAPE", widget.parent.ownerDocument.defaultView); yield onHidden; + yield ruleEditor.rule._applyingModifications; - yield waitForSuccess(() => { - return content.getComputedStyle(content.document.body).animationTimingFunction === "cubic-bezier(0, 0, 1, 1)"; - }, "Waiting for the change to be reverted on the element"); + yield waitForComputedStyleProperty("body", null, "animation-timing-function", + "cubic-bezier(0, 0, 1, 1)"); + is(propEditor.valueSpan.textContent, "linear", + "Got expected property value."); +} + +function* testPressingEscapeRevertsChangesAndDisables(view) { + let ruleEditor = getRuleViewRuleEditor(view, 1); + let propEditor = ruleEditor.rule.textProps[0].editor; + let swatch = propEditor.valueSpan.querySelector(".ruleview-bezierswatch"); + let bezierTooltip = view.tooltips.cubicBezier; + + info("Disabling animation-timing-function property"); + propEditor.enable.click(); + yield ruleEditor.rule._applyingModifications; + + ok(propEditor.element.classList.contains("ruleview-overridden"), + "property is overridden."); + is(propEditor.enable.style.visibility, "visible", + "property enable checkbox is visible."); + ok(!propEditor.enable.getAttribute("checked"), + "property enable checkbox is not checked."); + ok(!propEditor.prop.enabled, + "animation-timing-function property is disabled."); + let newValue = yield getRulePropertyValue("animation-timing-function"); + is(newValue, "", "animation-timing-function should have been unset."); + + let onShown = bezierTooltip.tooltip.once("shown"); + swatch.click(); + yield onShown; + + ok(!propEditor.element.classList.contains("ruleview-overridden"), + "property overridden is not displayed."); + is(propEditor.enable.style.visibility, "hidden", + "property enable checkbox is hidden."); + + let widget = yield bezierTooltip.widget; + info("Simulating a change of curve in the widget"); + widget.coordinates = [0.1, 2, 0.9, -1]; + yield ruleEditor.rule._applyingModifications; + + info("Pressing ESCAPE to close the tooltip"); + let onHidden = bezierTooltip.tooltip.once("hidden"); + EventUtils.sendKey("ESCAPE", widget.parent.ownerDocument.defaultView); + yield onHidden; + yield ruleEditor.rule._applyingModifications; + + ok(propEditor.element.classList.contains("ruleview-overridden"), + "property is overridden."); + is(propEditor.enable.style.visibility, "visible", + "property enable checkbox is visible."); + ok(!propEditor.enable.getAttribute("checked"), + "property enable checkbox is not checked."); + ok(!propEditor.prop.enabled, + "animation-timing-function property is disabled."); + newValue = yield getRulePropertyValue("animation-timing-function"); + is(newValue, "", "animation-timing-function should have been unset."); + is(propEditor.valueSpan.textContent, "linear", + "Got expected property value."); +} + +function* getRulePropertyValue(name) { + let propValue = yield executeInContent("Test:GetRulePropertyValue", { + styleSheetIndex: 0, + ruleIndex: 0, + name: name + }); + return propValue; } diff --git a/browser/devtools/styleinspector/test/browser_ruleview_filtereditor-revert-on-ESC.js b/browser/devtools/styleinspector/test/browser_ruleview_filtereditor-revert-on-ESC.js index 7f125b8a749..ca74c2cfa9c 100644 --- a/browser/devtools/styleinspector/test/browser_ruleview_filtereditor-revert-on-ESC.js +++ b/browser/devtools/styleinspector/test/browser_ruleview_filtereditor-revert-on-ESC.js @@ -3,36 +3,102 @@ "use strict"; -// Tests the Filter Editor Tooltip reverting changes on ESC +// Tests that changes made to the Filter Editor Tooltip are reverted when +// ESC is pressed const TEST_URL = TEST_URL_ROOT + "doc_filter.html"; add_task(function*() { yield addTab(TEST_URL); + let {view} = yield openRuleView(); + yield testPressingEscapeRevertsChanges(view); + yield testPressingEscapeRevertsChangesAndDisables(view); +}); - let {toolbox, inspector, view} = yield openRuleView(); - - info("Getting the filter swatch element"); - let swatch = getRuleViewProperty(view, "body", "filter").valueSpan - .querySelector(".ruleview-filterswatch"); - +function* testPressingEscapeRevertsChanges(view) { + let ruleEditor = getRuleViewRuleEditor(view, 1); + let propEditor = ruleEditor.rule.textProps[0].editor; + let swatch = propEditor.valueSpan.querySelector(".ruleview-filterswatch"); let filterTooltip = view.tooltips.filterEditor; + let onShow = filterTooltip.tooltip.once("shown"); swatch.click(); yield onShow; let widget = yield filterTooltip.widget; - widget.setCssValue("blur(2px)"); - yield waitForComputedStyleProperty("body", null, "filter", "blur(2px)"); + yield ruleEditor.rule._applyingModifications; - ok(true, "Changes previewed on the element"); + yield waitForComputedStyleProperty("body", null, "filter", "blur(2px)"); + is(propEditor.valueSpan.textContent, "blur(2px)", + "Got expected property value."); info("Pressing ESCAPE to close the tooltip"); EventUtils.sendKey("ESCAPE", widget.styleWindow); + yield ruleEditor.rule._applyingModifications; - yield waitForSuccess(() => { - const computed = content.getComputedStyle(content.document.body); - return computed.filter === "blur(2px) contrast(2)"; - }, "Waiting for the change to be reverted on the element"); -}); + yield waitForComputedStyleProperty("body", null, "filter", + "blur(2px) contrast(2)"); + is(propEditor.valueSpan.textContent, "blur(2px) contrast(2)", + "Got expected property value."); +} + +function* testPressingEscapeRevertsChangesAndDisables(view) { + let ruleEditor = getRuleViewRuleEditor(view, 1); + let propEditor = ruleEditor.rule.textProps[0].editor; + let swatch = propEditor.valueSpan.querySelector(".ruleview-filterswatch"); + let filterTooltip = view.tooltips.filterEditor; + + info("Disabling filter property"); + propEditor.enable.click(); + yield ruleEditor.rule._applyingModifications; + + ok(propEditor.element.classList.contains("ruleview-overridden"), + "property is overridden."); + is(propEditor.enable.style.visibility, "visible", + "property enable checkbox is visible."); + ok(!propEditor.enable.getAttribute("checked"), + "property enable checkbox is not checked."); + ok(!propEditor.prop.enabled, + "filter property is disabled."); + let newValue = yield getRulePropertyValue("filter"); + is(newValue, "", "filter should have been unset."); + + let onShow = filterTooltip.tooltip.once("shown"); + swatch.click(); + yield onShow; + + ok(!propEditor.element.classList.contains("ruleview-overridden"), + "property overridden is not displayed."); + is(propEditor.enable.style.visibility, "hidden", + "property enable checkbox is hidden."); + + let widget = yield filterTooltip.widget; + widget.setCssValue("blur(2px)"); + yield ruleEditor.rule._applyingModifications; + + info("Pressing ESCAPE to close the tooltip"); + EventUtils.sendKey("ESCAPE", widget.styleWindow); + yield ruleEditor.rule._applyingModifications; + + ok(propEditor.element.classList.contains("ruleview-overridden"), + "property is overridden."); + is(propEditor.enable.style.visibility, "visible", + "property enable checkbox is visible."); + ok(!propEditor.enable.getAttribute("checked"), + "property enable checkbox is not checked."); + ok(!propEditor.prop.enabled, "filter property is disabled."); + newValue = yield getRulePropertyValue("filter"); + is(newValue, "", "filter should have been unset."); + is(propEditor.valueSpan.textContent, "blur(2px) contrast(2)", + "Got expected property value."); +} + +function* getRulePropertyValue(name) { + let propValue = yield executeInContent("Test:GetRulePropertyValue", { + styleSheetIndex: 0, + ruleIndex: 0, + name: name + }); + return propValue; +} From 55980fb3d75597f67e57dad0f428ccad85538498 Mon Sep 17 00:00:00 2001 From: Jordan Santell Date: Thu, 16 Jul 2015 13:28:30 -0700 Subject: [PATCH 18/28] Bug 1163763 - L10N-ify many strings that didn't make previous uplifts for performance tools, and consolidate strings into one tool rather than profiler and timeline. r=vp --- browser/devtools/definitions.js | 14 +- .../devtools/performance/modules/global.js | 5 +- .../performance/modules/logic/marker-utils.js | 68 +++---- .../devtools/performance/modules/markers.js | 28 +-- .../performance/modules/widgets/graphs.js | 6 +- .../modules/widgets/marker-details.js | 2 - .../devtools/performance/performance-view.js | 5 +- browser/devtools/performance/performance.xul | 134 +++++++------- .../test/browser_perf-loading-01.js | 8 +- .../performance/views/optimizations-list.js | 2 +- .../devtools/performance/views/recordings.js | 9 +- .../browser/devtools/markers.properties | 80 +++++++++ .../chrome/browser/devtools/performance.dtd | 157 +++++++++++++++++ ...iler.properties => performance.properties} | 91 ++++++---- .../chrome/browser/devtools/profiler.dtd | 166 ------------------ .../chrome/browser/devtools/timeline.dtd | 43 ----- .../browser/devtools/timeline.properties | 79 --------- browser/locales/jar.mn | 7 +- 18 files changed, 425 insertions(+), 479 deletions(-) create mode 100644 browser/locales/en-US/chrome/browser/devtools/markers.properties create mode 100644 browser/locales/en-US/chrome/browser/devtools/performance.dtd rename browser/locales/en-US/chrome/browser/devtools/{profiler.properties => performance.properties} (63%) delete mode 100644 browser/locales/en-US/chrome/browser/devtools/profiler.dtd delete mode 100644 browser/locales/en-US/chrome/browser/devtools/timeline.dtd delete mode 100644 browser/locales/en-US/chrome/browser/devtools/timeline.properties diff --git a/browser/devtools/definitions.js b/browser/devtools/definitions.js index f2747c2dedf..319b6f875bf 100644 --- a/browser/devtools/definitions.js +++ b/browser/devtools/definitions.js @@ -33,13 +33,13 @@ const styleEditorProps = "chrome://browser/locale/devtools/styleeditor.propertie const shaderEditorProps = "chrome://browser/locale/devtools/shadereditor.properties"; const canvasDebuggerProps = "chrome://browser/locale/devtools/canvasdebugger.properties"; const webAudioEditorProps = "chrome://browser/locale/devtools/webaudioeditor.properties"; -const profilerProps = "chrome://browser/locale/devtools/profiler.properties"; +const performanceProps = "chrome://browser/locale/devtools/performance.properties"; const netMonitorProps = "chrome://browser/locale/devtools/netmonitor.properties"; const storageProps = "chrome://browser/locale/devtools/storage.properties"; const scratchpadProps = "chrome://browser/locale/devtools/scratchpad.properties"; loader.lazyGetter(this, "toolboxStrings", () => Services.strings.createBundle(toolboxProps)); -loader.lazyGetter(this, "profilerStrings",() => Services.strings.createBundle(profilerProps)); +loader.lazyGetter(this, "performanceStrings",() => Services.strings.createBundle(performanceProps)); loader.lazyGetter(this, "webConsoleStrings", () => Services.strings.createBundle(webConsoleProps)); loader.lazyGetter(this, "debuggerStrings", () => Services.strings.createBundle(debuggerProps)); loader.lazyGetter(this, "styleEditorStrings", () => Services.strings.createBundle(styleEditorProps)); @@ -254,14 +254,14 @@ Tools.performance = { highlightedicon: "chrome://browser/skin/devtools/tool-profiler-active.svg", url: "chrome://browser/content/devtools/performance.xul", visibilityswitch: "devtools.performance.enabled", - label: l10n("profiler.label2", profilerStrings), - panelLabel: l10n("profiler.panelLabel2", profilerStrings), + label: l10n("performance.label", performanceStrings), + panelLabel: l10n("performance.panelLabel", performanceStrings), get tooltip() { - return l10n("profiler.tooltip3", profilerStrings, + return l10n("performance.tooltip", performanceStrings, "Shift+" + functionkey(this.key)); }, - accesskey: l10n("profiler.accesskey", profilerStrings), - key: l10n("profiler.commandkey2", profilerStrings), + accesskey: l10n("performance.accesskey", performanceStrings), + key: l10n("performance.commandkey", performanceStrings), modifiers: "shift", inMenu: true, diff --git a/browser/devtools/performance/modules/global.js b/browser/devtools/performance/modules/global.js index 2038a5fefe5..1da0b762622 100644 --- a/browser/devtools/performance/modules/global.js +++ b/browser/devtools/performance/modules/global.js @@ -7,11 +7,10 @@ const { ViewHelpers } = require("resource:///modules/devtools/ViewHelpers.jsm"); /** * Localization convenience methods. - + TODO: merge these into a single file: Bug 1082695. */ const L10N = new ViewHelpers.MultiL10N([ - "chrome://browser/locale/devtools/timeline.properties", - "chrome://browser/locale/devtools/profiler.properties" + "chrome://browser/locale/devtools/markers.properties", + "chrome://browser/locale/devtools/performance.properties" ]); /** diff --git a/browser/devtools/performance/modules/logic/marker-utils.js b/browser/devtools/performance/modules/logic/marker-utils.js index ab05941efeb..0a2208e3ba3 100644 --- a/browser/devtools/performance/modules/logic/marker-utils.js +++ b/browser/devtools/performance/modules/logic/marker-utils.js @@ -93,12 +93,8 @@ function getMarkerFields (marker) { // If blueprint.fields is a function, use that if (typeof blueprint.fields === "function") { let fields = blueprint.fields(marker); - // Add a ":" to the label since the localization files contain the ":" - // if not present. This should be changed, ugh. return Object.keys(fields || []).map(label => { - // TODO revisit localization strings for markers bug 1163763 - let normalizedLabel = label.indexOf(":") !== -1 ? label : (label + ":"); - return { label: normalizedLabel, value: fields[label] }; + return { label, value: fields[label] }; }); } @@ -168,7 +164,7 @@ const DOM = { * @return {Element} */ buildDuration: function (doc, marker) { - let label = L10N.getStr("timeline.markerDetail.duration"); + let label = L10N.getStr("marker.field.duration"); let start = L10N.getFormatStrWithNumbers("timeline.tick", marker.start); let end = L10N.getFormatStrWithNumbers("timeline.tick", marker.end); let duration = L10N.getFormatStrWithNumbers("timeline.tick", marker.end - marker.start); @@ -217,7 +213,7 @@ const DOM = { let container = doc.createElement("vbox"); let labelName = doc.createElement("label"); labelName.className = "plain marker-details-labelname"; - labelName.setAttribute("value", L10N.getStr(`timeline.markerDetail.${type}`)); + labelName.setAttribute("value", L10N.getStr(`marker.field.${type}`)); container.setAttribute("type", type); container.className = "marker-details-stack"; container.appendChild(labelName); @@ -235,7 +231,7 @@ const DOM = { let asyncBox = doc.createElement("hbox"); let asyncLabel = doc.createElement("label"); asyncLabel.className = "devtools-monospace"; - asyncLabel.setAttribute("value", L10N.getFormatStr("timeline.markerDetail.asyncStack", + asyncLabel.setAttribute("value", L10N.getFormatStr("marker.field.asyncStack", frame.asyncCause)); asyncBox.appendChild(asyncLabel); container.appendChild(asyncBox); @@ -278,7 +274,7 @@ const DOM = { if (!displayName && !url) { let label = doc.createElement("label"); - label.setAttribute("value", L10N.getStr("timeline.markerDetail.unknownFrame")); + label.setAttribute("value", L10N.getStr("marker.value.unknownFrame")); hbox.appendChild(label); } @@ -301,18 +297,19 @@ const DOM = { * markers that are considered "from content" should be labeled here. */ const JS_MARKER_MAP = { - " + + + diff --git a/browser/devtools/performance/test/head.js b/browser/devtools/performance/test/head.js index 3bb0dcb0a2a..f46bb96cf1f 100644 --- a/browser/devtools/performance/test/head.js +++ b/browser/devtools/performance/test/head.js @@ -24,6 +24,7 @@ const FRAME_SCRIPT_UTILS_URL = "chrome://browser/content/devtools/frame-script-u const EXAMPLE_URL = "http://example.com/browser/browser/devtools/performance/test/"; const SIMPLE_URL = EXAMPLE_URL + "doc_simple-test.html"; const MARKERS_URL = EXAMPLE_URL + "doc_markers.html"; +const ALLOCS_URL = EXAMPLE_URL + "doc_allocs.html"; const MEMORY_SAMPLE_PROB_PREF = "devtools.performance.memory.sample-probability"; const MEMORY_MAX_LOG_LEN_PREF = "devtools.performance.memory.max-log-length"; diff --git a/toolkit/devtools/shared/memory.js b/toolkit/devtools/shared/memory.js index aa6ffb33a7c..e7039a1dc77 100644 --- a/toolkit/devtools/shared/memory.js +++ b/toolkit/devtools/shared/memory.js @@ -95,7 +95,7 @@ let Memory = exports.Memory = Class({ _clearDebuggees: function() { if (this._dbg) { - if (this.dbg.memory.trackingAllocationSites) { + if (this.isRecordingAllocations()) { this.dbg.memory.drainAllocationsLog(); } this._clearFrames(); @@ -104,7 +104,7 @@ let Memory = exports.Memory = Class({ }, _clearFrames: function() { - if (this.dbg.memory.trackingAllocationSites) { + if (this.isRecordingAllocations()) { this._frameCache.clearFrames(); } }, @@ -114,7 +114,7 @@ let Memory = exports.Memory = Class({ */ _onWindowReady: function({ isTopLevel }) { if (this.state == "attached") { - if (isTopLevel && this.dbg.memory.trackingAllocationSites) { + if (isTopLevel && this.isRecordingAllocations()) { this._clearDebuggees(); this._frameCache.initFrames(); } @@ -122,6 +122,14 @@ let Memory = exports.Memory = Class({ } }, + /** + * Returns a boolean indicating whether or not allocation + * sites are being tracked. + */ + isRecordingAllocations: function () { + return this.dbg.memory.trackingAllocationSites; + }, + /** * Take a census of the heap. See js/src/doc/Debugger/Debugger.Memory.md for * more information. @@ -146,8 +154,8 @@ let Memory = exports.Memory = Class({ * resetting the timer. */ startRecordingAllocations: expectState("attached", function(options = {}) { - if (this.dbg.memory.trackingAllocationSites) { - return Date.now(); + if (this.isRecordingAllocations()) { + return this._getCurrentTime(); } this._frameCache.initFrames(); @@ -171,13 +179,16 @@ let Memory = exports.Memory = Class({ } this.dbg.memory.trackingAllocationSites = true; - return Date.now(); + return this._getCurrentTime(); }, `starting recording allocations`), /** * Stop recording allocation sites. */ stopRecordingAllocations: expectState("attached", function() { + if (!this.isRecordingAllocations()) { + return this._getCurrentTime(); + } this.dbg.memory.trackingAllocationSites = false; this._clearFrames(); @@ -186,7 +197,7 @@ let Memory = exports.Memory = Class({ this._poller = null; } - return Date.now(); + return this._getCurrentTime(); }, `stopping recording allocations`), /** @@ -380,4 +391,12 @@ let Memory = exports.Memory = Class({ events.emit(this, "allocations", this.getAllocations()); this._poller.arm(); }, + + /** + * Accesses the docshell to return the current process time. + */ + _getCurrentTime: function () { + return (this.parent.isRootActor ? this.parent.docShell : this.parent.originalDocShell).now(); + }, + }); From edcc173ff35f301153d45460ace40b275f0c5a04 Mon Sep 17 00:00:00 2001 From: Dave Townsend Date: Tue, 4 Aug 2015 14:05:20 -0700 Subject: [PATCH 23/28] Bug 1190966: Ensure that the signature verification scan disables existing add-ons if the pref has been flipped in the meantime. r=rhelmer --- .../extensions/internal/XPIProvider.jsm | 12 +- .../extensions/internal/XPIProviderUtils.js | 6 +- .../test/xpcshell/test_signed_updatepref.js | 134 ++++++++++++++++++ .../test/xpcshell/xpcshell-shared.ini | 1 + .../extensions/test/xpcshell/xpcshell.ini | 1 - 5 files changed, 146 insertions(+), 8 deletions(-) create mode 100644 toolkit/mozapps/extensions/test/xpcshell/test_signed_updatepref.js diff --git a/toolkit/mozapps/extensions/internal/XPIProvider.jsm b/toolkit/mozapps/extensions/internal/XPIProvider.jsm index cb59cee344b..e4c6f987770 100644 --- a/toolkit/mozapps/extensions/internal/XPIProvider.jsm +++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm @@ -2513,13 +2513,13 @@ this.XPIProvider = { continue; let signedState = yield verifyBundleSignedState(addon._sourceBundle, addon); - if (signedState == addon.signedState) - continue; - addon.signedState = signedState; - AddonManagerPrivate.callAddonListeners("onPropertyChanged", - createWrapper(addon), - ["signedState"]); + if (signedState != addon.signedState) { + addon.signedState = signedState; + AddonManagerPrivate.callAddonListeners("onPropertyChanged", + createWrapper(addon), + ["signedState"]); + } let disabled = XPIProvider.updateAddonDisabledState(addon); if (disabled !== undefined) diff --git a/toolkit/mozapps/extensions/internal/XPIProviderUtils.js b/toolkit/mozapps/extensions/internal/XPIProviderUtils.js index c9dbd2e6938..063a76705fa 100644 --- a/toolkit/mozapps/extensions/internal/XPIProviderUtils.js +++ b/toolkit/mozapps/extensions/internal/XPIProviderUtils.js @@ -342,6 +342,8 @@ function DBAddonInternalPrototype() { this.applyCompatibilityUpdate = function(aUpdate, aSyncCompatibility) { + let wasCompatible = this.isCompatible; + this.targetApplications.forEach(function(aTargetApp) { aUpdate.targetApplications.forEach(function(aUpdateTarget) { if (aTargetApp.id == aUpdateTarget.id && (aSyncCompatibility || @@ -357,7 +359,9 @@ function DBAddonInternalPrototype() this.multiprocessCompatible = aUpdate.multiprocessCompatible; XPIDatabase.saveChanges(); } - XPIProvider.updateAddonDisabledState(this); + + if (wasCompatible != this.isCompatible) + XPIProvider.updateAddonDisabledState(this); }; this.toJSON = diff --git a/toolkit/mozapps/extensions/test/xpcshell/test_signed_updatepref.js b/toolkit/mozapps/extensions/test/xpcshell/test_signed_updatepref.js new file mode 100644 index 00000000000..30dcafc4c7f --- /dev/null +++ b/toolkit/mozapps/extensions/test/xpcshell/test_signed_updatepref.js @@ -0,0 +1,134 @@ +// Disable update security +Services.prefs.setBoolPref(PREF_EM_CHECK_UPDATE_SECURITY, false); + +const DATA = "data/signing_checks/"; +const ID = "test@tests.mozilla.org"; + +Components.utils.import("resource://testing-common/httpd.js"); +var gServer = new HttpServer(); +gServer.start(); + +gServer.registerPathHandler("/update.rdf", function(request, response) { + let updateData = {}; + updateData[ID] = [{ + version: "2.0", + targetApplications: [{ + id: "xpcshell@tests.mozilla.org", + minVersion: "4", + maxVersion: "6" + }] + }]; + + response.setStatusLine(request.httpVersion, 200, "OK"); + response.write(createUpdateRDF(updateData)); +}); + +const SERVER = "127.0.0.1:" + gServer.identity.primaryPort; +Services.prefs.setCharPref("extensions.update.background.url", "http://" + SERVER + "/update.rdf"); + +function verifySignatures() { + return new Promise(resolve => { + let observer = (subject, topic, data) => { + Services.obs.removeObserver(observer, "xpi-signature-changed"); + resolve(JSON.parse(data)); + } + Services.obs.addObserver(observer, "xpi-signature-changed", false); + + do_print("Verifying signatures"); + let XPIscope = Components.utils.import("resource://gre/modules/addons/XPIProvider.jsm"); + XPIscope.XPIProvider.verifySignatures(); + }); +} + +function run_test() { + createAppInfo("xpcshell@tests.mozilla.org", "XPCShell", "4", "4"); + + // Start and stop the manager to initialise everything in the profile before + // actual testing + startupManager(); + shutdownManager(); + + run_next_test(); +} + +// Updating the pref without changing the app version won't disable add-ons +// immediately but will after a signing check +add_task(function*() { + startupManager(); + + // Install the signed add-on + yield promiseInstallAllFiles([do_get_file(DATA + "unsigned_bootstrap_2.xpi")]); + + let addon = yield promiseAddonByID(ID); + do_check_neq(addon, null); + do_check_false(addon.appDisabled); + do_check_true(addon.isActive); + do_check_eq(addon.signedState, AddonManager.SIGNEDSTATE_MISSING); + + yield promiseShutdownManager(); + + Services.prefs.setBoolPref(PREF_XPI_SIGNATURES_REQUIRED, true); + + startupManager(); + + addon = yield promiseAddonByID(ID); + do_check_neq(addon, null); + do_check_false(addon.appDisabled); + do_check_true(addon.isActive); + do_check_eq(addon.signedState, AddonManager.SIGNEDSTATE_MISSING); + + // Update checks shouldn't affect the add-on + yield AddonManagerInternal.backgroundUpdateCheck(); + addon = yield promiseAddonByID(ID); + do_check_neq(addon, null); + do_check_false(addon.appDisabled); + do_check_true(addon.isActive); + do_check_eq(addon.signedState, AddonManager.SIGNEDSTATE_MISSING); + + let changes = yield verifySignatures(); + + do_check_eq(changes.disabled.length, 1); + do_check_eq(changes.disabled[0], ID); + + addon = yield promiseAddonByID(ID); + do_check_neq(addon, null); + do_check_true(addon.appDisabled); + do_check_false(addon.isActive); + do_check_eq(addon.signedState, AddonManager.SIGNEDSTATE_MISSING); + + addon.uninstall(); + + yield promiseShutdownManager(); +}); + +// Updating the pref with changing the app version will disable add-ons +// immediately +add_task(function*() { + Services.prefs.setBoolPref(PREF_XPI_SIGNATURES_REQUIRED, false); + startupManager(); + + // Install the signed add-on + yield promiseInstallAllFiles([do_get_file(DATA + "unsigned_bootstrap_2.xpi")]); + + let addon = yield promiseAddonByID(ID); + do_check_neq(addon, null); + do_check_false(addon.appDisabled); + do_check_true(addon.isActive); + do_check_eq(addon.signedState, AddonManager.SIGNEDSTATE_MISSING); + + yield promiseShutdownManager(); + + Services.prefs.setBoolPref(PREF_XPI_SIGNATURES_REQUIRED, true); + gAppInfo.version = 5.0 + startupManager(true); + + addon = yield promiseAddonByID(ID); + do_check_neq(addon, null); + do_check_true(addon.appDisabled); + do_check_false(addon.isActive); + do_check_eq(addon.signedState, AddonManager.SIGNEDSTATE_MISSING); + + addon.uninstall(); + + yield promiseShutdownManager(); +}); diff --git a/toolkit/mozapps/extensions/test/xpcshell/xpcshell-shared.ini b/toolkit/mozapps/extensions/test/xpcshell/xpcshell-shared.ini index 1d56a590c13..aeba8242b9b 100644 --- a/toolkit/mozapps/extensions/test/xpcshell/xpcshell-shared.ini +++ b/toolkit/mozapps/extensions/test/xpcshell/xpcshell-shared.ini @@ -237,6 +237,7 @@ fail-if = buildapp == "mulet" || os == "android" [test_pref_properties.js] [test_registry.js] [test_safemode.js] +[test_signed_updatepref.js] [test_signed_verify.js] [test_signed_inject.js] [test_signed_install.js] diff --git a/toolkit/mozapps/extensions/test/xpcshell/xpcshell.ini b/toolkit/mozapps/extensions/test/xpcshell/xpcshell.ini index 65a8c23ec84..df27e4da62c 100644 --- a/toolkit/mozapps/extensions/test/xpcshell/xpcshell.ini +++ b/toolkit/mozapps/extensions/test/xpcshell/xpcshell.ini @@ -26,5 +26,4 @@ skip-if = appname != "firefox" [test_XPIcancel.js] [test_XPIStates.js] - [include:xpcshell-shared.ini] From e3f5e1487e4b1201a5e43713c64998285ee0e78c Mon Sep 17 00:00:00 2001 From: vivek Date: Wed, 5 Aug 2015 00:47:08 +0300 Subject: [PATCH 24/28] Bug 1055264 - Fixed profile image update after sync now r=nalexander --- mobile/android/base/fxa/authenticator/AndroidFxAccount.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mobile/android/base/fxa/authenticator/AndroidFxAccount.java b/mobile/android/base/fxa/authenticator/AndroidFxAccount.java index 5232b7676ec..96ad92a366e 100644 --- a/mobile/android/base/fxa/authenticator/AndroidFxAccount.java +++ b/mobile/android/base/fxa/authenticator/AndroidFxAccount.java @@ -789,7 +789,7 @@ public class AndroidFxAccount { updateBundleValues(BUNDLE_KEY_PROFILE_JSON, resultData); Logger.info(LOG_TAG, "Profile JSON fetch succeeeded!"); FxAccountUtils.pii(LOG_TAG, "Profile JSON fetch returned: " + resultData); - LocalBroadcastManager.getInstance(context).sendBroadcast(makeDeletedAccountIntent()); + LocalBroadcastManager.getInstance(context).sendBroadcast(makeProfileJSONUpdatedIntent()); break; case Activity.RESULT_CANCELED: Logger.warn(LOG_TAG, "Failed to fetch profile JSON; ignoring."); From 7b75fba06e526e5500808db70c401225711f97a7 Mon Sep 17 00:00:00 2001 From: Mark Hammond Date: Wed, 5 Aug 2015 14:22:20 +1000 Subject: [PATCH 25/28] Bug 1190279 - fix UI for unverified FxA users in both hamburger menu and Sync prefs pane. r=oeger --- browser/base/content/browser-fxaccounts.js | 22 +- browser/base/content/test/general/browser.ini | 2 + .../test/general/browser_fxaccounts.js | 258 ++++++++++++++++++ .../test/general/fxa_profile_handler.sjs | 34 +++ .../components/preferences/in-content/sync.js | 1 + 5 files changed, 310 insertions(+), 7 deletions(-) create mode 100644 browser/base/content/test/general/browser_fxaccounts.js create mode 100644 browser/base/content/test/general/fxa_profile_handler.sjs diff --git a/browser/base/content/browser-fxaccounts.js b/browser/base/content/browser-fxaccounts.js index b45d6fb4801..f7f1092d80a 100644 --- a/browser/base/content/browser-fxaccounts.js +++ b/browser/base/content/browser-fxaccounts.js @@ -33,6 +33,7 @@ let gFxAccounts = { "weave:service:setup-complete", "weave:ui:login:error", "fxa-migration:state-changed", + this.FxAccountsCommon.ONLOGIN_NOTIFICATION, this.FxAccountsCommon.ONVERIFIED_NOTIFICATION, this.FxAccountsCommon.ONLOGOUT_NOTIFICATION, "weave:notification:removed", @@ -222,10 +223,11 @@ let gFxAccounts = { this.updateMigrationNotification(); }, + // Note that updateAppMenuItem() returns a Promise that's only used by tests. updateAppMenuItem: function () { if (this._migrationInfo) { this.updateAppMenuItemForMigration(); - return; + return Promise.resolve(); } let profileInfoEnabled = false; @@ -241,7 +243,7 @@ let gFxAccounts = { // state once migration is complete. this.panelUIFooter.hidden = true; this.panelUIFooter.removeAttribute("fxastatus"); - return; + return Promise.resolve(); } this.panelUIFooter.hidden = false; @@ -311,12 +313,18 @@ let gFxAccounts = { } } - // Calling getSignedInUserProfile() without a user logged in causes log - // noise that looks like an actual error... - fxAccounts.getSignedInUser().then(userData => { + return fxAccounts.getSignedInUser().then(userData => { // userData may be null here when the user is not signed-in, but that's expected updateWithUserData(userData); - return userData ? fxAccounts.getSignedInUserProfile() : null; + // unverified users cause us to spew log errors fetching an OAuth token + // to fetch the profile, so don't even try in that case. + if (!userData || !userData.verified || !profileInfoEnabled) { + return null; // don't even try to grab the profile. + } + return fxAccounts.getSignedInUserProfile().catch(err => { + // Not fetching the profile is sad but the FxA logs will already have noise. + return null; + }); }).then(profile => { if (!profile) { return; @@ -327,7 +335,7 @@ let gFxAccounts = { // The most likely scenario is a user logged out, so reflect that. // Bug 995134 calls for better errors so we could retry if we were // sure this was the failure reason. - this.FxAccountsCommon.log.error("Error updating FxA profile", error); + this.FxAccountsCommon.log.error("Error updating FxA account info", error); updateWithUserData(null); }); }, diff --git a/browser/base/content/test/general/browser.ini b/browser/base/content/test/general/browser.ini index d0431cec30b..4b19883871c 100644 --- a/browser/base/content/test/general/browser.ini +++ b/browser/base/content/test/general/browser.ini @@ -303,6 +303,8 @@ skip-if = true # browser_drag.js is disabled, as it needs to be updated for the [browser_focusonkeydown.js] [browser_fullscreen-window-open.js] skip-if = buildapp == 'mulet' || e10s || os == "linux" # Bug 933103 - mochitest's EventUtils.synthesizeMouse functions not e10s friendly. Linux: Intermittent failures - bug 941575. +[browser_fxaccounts.js] +support-files = fxa_profile_handler.sjs [browser_fxa_migrate.js] [browser_fxa_oauth.js] [browser_fxa_web_channel.js] diff --git a/browser/base/content/test/general/browser_fxaccounts.js b/browser/base/content/test/general/browser_fxaccounts.js new file mode 100644 index 00000000000..aef181860f6 --- /dev/null +++ b/browser/base/content/test/general/browser_fxaccounts.js @@ -0,0 +1,258 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ */ + +let {Log} = Cu.import("resource://gre/modules/Log.jsm", {}); +let {Task} = Cu.import("resource://gre/modules/Task.jsm", {}); +let {fxAccounts} = Cu.import("resource://gre/modules/FxAccounts.jsm", {}); +let FxAccountsCommon = {}; +Cu.import("resource://gre/modules/FxAccountsCommon.js", FxAccountsCommon); + +const TEST_ROOT = "http://example.com/browser/browser/base/content/test/general/"; + +// instrument gFxAccounts to send observer notifications when it's done +// what it does. +(function() { + let unstubs = {}; // The original functions we stub out. + + // The stub functions. + let stubs = { + updateAppMenuItem: function() { + return unstubs['updateAppMenuItem'].call(gFxAccounts).then(() => { + Services.obs.notifyObservers(null, "test:browser_fxaccounts:updateAppMenuItem", null); + }); + }, + // Opening preferences is trickier than it should be as leaks are reported + // due to the promises it fires off at load time and there's no clear way to + // know when they are done. + // So just ensure openPreferences is called rather than whether it opens. + openPreferences: function() { + Services.obs.notifyObservers(null, "test:browser_fxaccounts:openPreferences", null); + } + }; + + for (let name in stubs) { + unstubs[name] = gFxAccounts[name]; + gFxAccounts[name] = stubs[name]; + } + // and undo our damage at the end. + registerCleanupFunction(() => { + for (let name in unstubs) { + gFxAccounts[name] = unstubs[name]; + } + stubs = unstubs = null; + }); +})(); + +// Other setup/cleanup +let newTab; + +Services.prefs.setCharPref("identity.fxaccounts.remote.signup.uri", + TEST_ROOT + "accounts_testRemoteCommands.html"); + +registerCleanupFunction(() => { + Services.prefs.clearUserPref("identity.fxaccounts.remote.signup.uri"); + Services.prefs.clearUserPref("identity.fxaccounts.remote.profile.uri"); + gBrowser.removeTab(newTab); +}); + +add_task(function* initialize() { + // Set a new tab with something other than about:blank, so it doesn't get reused. + // We must wait for it to load or the promiseTabOpen() call in the next test + // gets confused. + newTab = gBrowser.selectedTab = gBrowser.addTab("about:mozilla", {animate: false}); + yield promiseTabLoaded(newTab); +}); + +// The elements we care about. +let panelUILabel = document.getElementById("PanelUI-fxa-label"); +let panelUIStatus = document.getElementById("PanelUI-fxa-status"); +let panelUIFooter = document.getElementById("PanelUI-footer-fxa"); + +// The tests +add_task(function* test_nouser() { + let user = yield fxAccounts.getSignedInUser(); + Assert.strictEqual(user, null, "start with no user signed in"); + let promiseUpdateDone = promiseObserver("test:browser_fxaccounts:updateAppMenuItem"); + Services.obs.notifyObservers(null, this.FxAccountsCommon.ONLOGOUT_NOTIFICATION, null); + yield promiseUpdateDone; + + // Check the world - the FxA footer area is visible as it is offering a signin. + Assert.ok(isFooterVisible()) + + Assert.equal(panelUILabel.getAttribute("label"), panelUIStatus.getAttribute("defaultlabel")); + Assert.ok(!panelUIStatus.hasAttribute("tooltiptext"), "no tooltip when signed out"); + Assert.ok(!panelUIFooter.hasAttribute("fxastatus"), "no fxsstatus when signed out"); + Assert.ok(!panelUIFooter.hasAttribute("fxaprofileimage"), "no fxaprofileimage when signed out"); + + let promiseOpen = promiseTabOpen("about:accounts?entryPoint=menupanel"); + panelUIStatus.click(); + yield promiseOpen; +}); + +/* +XXX - Bug 1191162 - need a better hawk mock story or this will leak in debug builds. + +add_task(function* test_unverifiedUser() { + let promiseUpdateDone = promiseObserver("test:browser_fxaccounts:updateAppMenuItem"); + yield setSignedInUser(false); // this will fire the observer that does the update. + yield promiseUpdateDone; + + // Check the world. + Assert.ok(isFooterVisible()) + + Assert.equal(panelUILabel.getAttribute("label"), "foo@example.com"); + Assert.equal(panelUIStatus.getAttribute("tooltiptext"), + panelUIStatus.getAttribute("signedinTooltiptext")); + Assert.equal(panelUIFooter.getAttribute("fxastatus"), "signedin"); + let promisePreferencesOpened = promiseObserver("test:browser_fxaccounts:openPreferences"); + panelUIStatus.click(); + yield promisePreferencesOpened + yield signOut(); +}); +*/ + +add_task(function* test_verifiedUserEmptyProfile() { + // We see 2 updateAppMenuItem() calls - one for the signedInUser and one after + // we first fetch the profile. We want them both to fire or we aren't testing + // the state we think we are testing. + let promiseUpdateDone = promiseObserver("test:browser_fxaccounts:updateAppMenuItem", 2); + configureProfileURL({}); // successful but empty profile. + yield setSignedInUser(true); // this will fire the observer that does the update. + yield promiseUpdateDone; + + // Check the world. + Assert.ok(isFooterVisible()) + Assert.equal(panelUILabel.getAttribute("label"), "foo@example.com"); + Assert.equal(panelUIStatus.getAttribute("tooltiptext"), + panelUIStatus.getAttribute("signedinTooltiptext")); + Assert.equal(panelUIFooter.getAttribute("fxastatus"), "signedin"); + + let promisePreferencesOpened = promiseObserver("test:browser_fxaccounts:openPreferences"); + panelUIStatus.click(); + yield promisePreferencesOpened; + yield signOut(); +}); + +add_task(function* test_verifiedUserDisplayName() { + let promiseUpdateDone = promiseObserver("test:browser_fxaccounts:updateAppMenuItem", 2); + configureProfileURL({ displayName: "Test User Display Name" }); + yield setSignedInUser(true); // this will fire the observer that does the update. + yield promiseUpdateDone; + + Assert.ok(isFooterVisible()) + Assert.equal(panelUILabel.getAttribute("label"), "Test User Display Name"); + Assert.equal(panelUIStatus.getAttribute("tooltiptext"), + panelUIStatus.getAttribute("signedinTooltiptext")); + Assert.equal(panelUIFooter.getAttribute("fxastatus"), "signedin"); + yield signOut(); +}); + +add_task(function* test_verifiedUserProfileFailure() { + // profile failure means only one observer fires. + let promiseUpdateDone = promiseObserver("test:browser_fxaccounts:updateAppMenuItem", 1); + configureProfileURL(null, 500); + yield setSignedInUser(true); // this will fire the observer that does the update. + yield promiseUpdateDone; + + Assert.ok(isFooterVisible()) + Assert.equal(panelUILabel.getAttribute("label"), "foo@example.com"); + Assert.equal(panelUIStatus.getAttribute("tooltiptext"), + panelUIStatus.getAttribute("signedinTooltiptext")); + Assert.equal(panelUIFooter.getAttribute("fxastatus"), "signedin"); + yield signOut(); +}); + +// Helpers. +function isFooterVisible() { + let style = window.getComputedStyle(panelUIFooter); + return style.getPropertyValue("display") == "flex"; +} + +function configureProfileURL(profile, responseStatus = 200) { + let responseBody = profile ? JSON.stringify(profile) : ""; + let url = TEST_ROOT + "fxa_profile_handler.sjs?" + + "responseStatus=" + responseStatus + + "responseBody=" + responseBody + + // This is a bit cheeky - the FxA code will just append "/profile" + // to the preference value. We arrange for this to be seen by our + //.sjs as part of the query string. + "&path="; + + Services.prefs.setCharPref("identity.fxaccounts.remote.profile.uri", url); +} + +function promiseObserver(topic, count = 1) { + return new Promise(resolve => { + let obs = (subject, topic, data) => { + if (--count == 0) { + Services.obs.removeObserver(obs, topic); + resolve(subject); + } + } + Services.obs.addObserver(obs, topic, false); + }); +} + +// Stolen from browser_aboutHome.js +function promiseWaitForEvent(node, type, capturing) { + return new Promise((resolve) => { + node.addEventListener(type, function listener(event) { + node.removeEventListener(type, listener, capturing); + resolve(event); + }, capturing); + }); +} + +let promiseTabOpen = Task.async(function*(urlBase) { + info("Waiting for tab to open..."); + let event = yield promiseWaitForEvent(gBrowser.tabContainer, "TabOpen", true); + let tab = event.target; + yield promiseTabLoadEvent(tab); + ok(tab.linkedBrowser.currentURI.spec.startsWith(urlBase), + "Got " + tab.linkedBrowser.currentURI.spec + ", expecting " + urlBase); + let whenUnloaded = promiseTabUnloaded(tab); + gBrowser.removeTab(tab); + yield whenUnloaded; +}); + +function promiseTabUnloaded(tab) +{ + return new Promise(resolve => { + info("Wait for tab to unload"); + function handle(event) { + tab.linkedBrowser.removeEventListener("unload", handle, true); + info("Got unload event"); + resolve(event); + } + tab.linkedBrowser.addEventListener("unload", handle, true, true); + }); +} + +// FxAccounts helpers. +function setSignedInUser(verified) { + let data = { + email: "foo@example.com", + uid: "1234@lcip.org", + assertion: "foobar", + sessionToken: "dead", + kA: "beef", + kB: "cafe", + verified: verified, + + oauthTokens: { + // a token for the profile server. + profile: "key value", + } + } + return fxAccounts.setSignedInUser(data); +} + +let signOut = Task.async(function* () { + // This test needs to make sure that any updates for the logout have + // completed before starting the next test, or we see the observer + // notifications get out of sync. + let promiseUpdateDone = promiseObserver("test:browser_fxaccounts:updateAppMenuItem"); + // we always want a "localOnly" signout here... + yield fxAccounts.signOut(true); + yield promiseUpdateDone; +}); diff --git a/browser/base/content/test/general/fxa_profile_handler.sjs b/browser/base/content/test/general/fxa_profile_handler.sjs new file mode 100644 index 00000000000..7160b76d0b2 --- /dev/null +++ b/browser/base/content/test/general/fxa_profile_handler.sjs @@ -0,0 +1,34 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ */ + +// This is basically an echo server! +// We just grab responseStatus and responseBody query params! + +function reallyHandleRequest(request, response) { + var query = "?" + request.queryString; + + var responseStatus = 200; + var match = /responseStatus=([^&]*)/.exec(query); + if (match) { + responseStatus = parseInt(match[1]); + } + + var responseBody = ""; + match = /responseBody=([^&]*)/.exec(query); + if (match) { + responseBody = decodeURIComponent(match[1]); + } + + response.setStatusLine("1.0", responseStatus, "OK"); + response.write(responseBody); +} + +function handleRequest(request, response) +{ + try { + reallyHandleRequest(request, response); + } catch (e) { + response.setStatusLine("1.0", 500, "NotOK"); + response.write("Error handling request: " + e); + } +} diff --git a/browser/components/preferences/in-content/sync.js b/browser/components/preferences/in-content/sync.js index df4a677c6e0..c4b4993f5c8 100644 --- a/browser/components/preferences/in-content/sync.js +++ b/browser/components/preferences/in-content/sync.js @@ -119,6 +119,7 @@ let gSyncPane = { "weave:service:setup-complete", "weave:service:logout:finish", FxAccountsCommon.ONVERIFIED_NOTIFICATION, + FxAccountsCommon.ONLOGIN_NOTIFICATION, FxAccountsCommon.ON_PROFILE_CHANGE_NOTIFICATION, ]; let migrateTopic = "fxa-migration:state-changed"; From 8443c6d0df45bc2388d5d9bb1c54beda4f6df2ca Mon Sep 17 00:00:00 2001 From: Andrei Oprea Date: Tue, 4 Aug 2015 22:22:27 -0700 Subject: [PATCH 26/28] Bug 1183617 - Implement updated design of contact buttons --- .../components/loop/content/css/contacts.css | 28 +++++++++++++++++++ browser/components/loop/content/css/panel.css | 7 ++--- .../components/loop/content/js/contacts.js | 28 ++++++++++--------- .../components/loop/content/js/contacts.jsx | 28 ++++++++++--------- .../en-US/chrome/browser/loop/loop.properties | 3 ++ 5 files changed, 63 insertions(+), 31 deletions(-) diff --git a/browser/components/loop/content/css/contacts.css b/browser/components/loop/content/css/contacts.css index 4d8579194c2..d6bbc53dd77 100644 --- a/browser/components/loop/content/css/contacts.css +++ b/browser/components/loop/content/css/contacts.css @@ -283,3 +283,31 @@ html[dir="rtl"] .contacts-gravatar-promo > .button-close { right: auto; left: 8px; } + +.contact-controls { + padding: 0 16px; +} + +.contact-controls > .button { + padding: .5em; + border: none; + border-radius: 5px; +} + +.button.primary { + background: #00A9DC; + color: #fff; +} + +.button.secondary { + background: #EBEBEB; + color: #4D4D4D; +} + +.contact-controls > .primary { + flex: 5; +} + +.contact-controls > .secondary { + flex: 3; +} diff --git a/browser/components/loop/content/css/panel.css b/browser/components/loop/content/css/panel.css index 175c26b74ad..fb0eeffdfe7 100644 --- a/browser/components/loop/content/css/panel.css +++ b/browser/components/loop/content/css/panel.css @@ -435,10 +435,7 @@ body { border-radius: 2px; min-height: 26px; font-size: 1.2rem; -} - -.button > .button-caption { - vertical-align: middle; + line-height: 1.2rem; } .button:hover { @@ -785,7 +782,7 @@ html[dir="rtl"] .settings-menu .dropdown-menu { font-size: 1rem; background-color: #fff; color: #666666; - padding: .5rem 1rem; + padding: .5rem 15px; } .footer .signin-details { diff --git a/browser/components/loop/content/js/contacts.js b/browser/components/loop/content/js/contacts.js index 9b2befe955a..e4f56d2a587 100644 --- a/browser/components/loop/content/js/contacts.js +++ b/browser/components/loop/content/js/contacts.js @@ -590,19 +590,6 @@ loop.contacts = (function(_, mozL10n) { return ( React.createElement("div", null, React.createElement("div", {className: "content-area"}, - React.createElement(ButtonGroup, null, - React.createElement(Button, {caption: this.state.importBusy - ? mozL10n.get("importing_contacts_progress_button") - : mozL10n.get("import_contacts_button2"), - disabled: this.state.importBusy, - onClick: this.handleImportButtonClick}, - React.createElement("div", {className: cx({"contact-import-spinner": true, - spinner: true, - busy: this.state.importBusy})}) - ), - React.createElement(Button, {caption: mozL10n.get("new_contact_button"), - onClick: this.handleAddContactButtonClick}) - ), showFilter ? React.createElement("input", {className: "contact-filter", placeholder: mozL10n.get("contacts_search_placesholder"), @@ -620,6 +607,21 @@ loop.contacts = (function(_, mozL10n) { shownContacts.blocked ? shownContacts.blocked.sort(this.sortContacts).map(viewForItem) : null + ), + React.createElement(ButtonGroup, {additionalClass: "contact-controls"}, + React.createElement(Button, {additionalClass: "secondary", + caption: this.state.importBusy + ? mozL10n.get("importing_contacts_progress_button") + : mozL10n.get("import_contacts_button3"), + disabled: this.state.importBusy, + onClick: this.handleImportButtonClick}, + React.createElement("div", {className: cx({"contact-import-spinner": true, + spinner: true, + busy: this.state.importBusy})}) + ), + React.createElement(Button, {additionalClass: "primary", + caption: mozL10n.get("new_contact_button"), + onClick: this.handleAddContactButtonClick}) ) ) ); diff --git a/browser/components/loop/content/js/contacts.jsx b/browser/components/loop/content/js/contacts.jsx index bbe612c0cec..674c8a966d2 100644 --- a/browser/components/loop/content/js/contacts.jsx +++ b/browser/components/loop/content/js/contacts.jsx @@ -590,19 +590,6 @@ loop.contacts = (function(_, mozL10n) { return (
- - - +
); } diff --git a/browser/locales/en-US/chrome/browser/loop/loop.properties b/browser/locales/en-US/chrome/browser/loop/loop.properties index 436529e43e8..52efe6e1d28 100644 --- a/browser/locales/en-US/chrome/browser/loop/loop.properties +++ b/browser/locales/en-US/chrome/browser/loop/loop.properties @@ -117,6 +117,9 @@ valid_email_text_description=Please enter a valid email address ## panel. add_or_import_contact_title=Add or Import Contact import_contacts_button2=Import from Google +## LOCALIZATION NOTE (import_contacts_button3): Text for button used to import +## contacts into the contact list. +import_contacts_button3=Import importing_contacts_progress_button=Importing… import_contacts_failure_message=Some contacts could not be imported. Please try again. ## LOCALIZATION NOTE(import_contacts_success_message): Success notification message From 988ef431a7089f37982875e2cdd06806bc56ecde Mon Sep 17 00:00:00 2001 From: "Carsten \"Tomcat\" Book" Date: Wed, 5 Aug 2015 07:47:37 +0200 Subject: [PATCH 27/28] Backed out changeset ad37329e81ce (bug 1190279) for test failures in browser_fxaccounts.js --- browser/base/content/browser-fxaccounts.js | 22 +- browser/base/content/test/general/browser.ini | 2 - .../test/general/browser_fxaccounts.js | 258 ------------------ .../test/general/fxa_profile_handler.sjs | 34 --- .../components/preferences/in-content/sync.js | 1 - 5 files changed, 7 insertions(+), 310 deletions(-) delete mode 100644 browser/base/content/test/general/browser_fxaccounts.js delete mode 100644 browser/base/content/test/general/fxa_profile_handler.sjs diff --git a/browser/base/content/browser-fxaccounts.js b/browser/base/content/browser-fxaccounts.js index f7f1092d80a..b45d6fb4801 100644 --- a/browser/base/content/browser-fxaccounts.js +++ b/browser/base/content/browser-fxaccounts.js @@ -33,7 +33,6 @@ let gFxAccounts = { "weave:service:setup-complete", "weave:ui:login:error", "fxa-migration:state-changed", - this.FxAccountsCommon.ONLOGIN_NOTIFICATION, this.FxAccountsCommon.ONVERIFIED_NOTIFICATION, this.FxAccountsCommon.ONLOGOUT_NOTIFICATION, "weave:notification:removed", @@ -223,11 +222,10 @@ let gFxAccounts = { this.updateMigrationNotification(); }, - // Note that updateAppMenuItem() returns a Promise that's only used by tests. updateAppMenuItem: function () { if (this._migrationInfo) { this.updateAppMenuItemForMigration(); - return Promise.resolve(); + return; } let profileInfoEnabled = false; @@ -243,7 +241,7 @@ let gFxAccounts = { // state once migration is complete. this.panelUIFooter.hidden = true; this.panelUIFooter.removeAttribute("fxastatus"); - return Promise.resolve(); + return; } this.panelUIFooter.hidden = false; @@ -313,18 +311,12 @@ let gFxAccounts = { } } - return fxAccounts.getSignedInUser().then(userData => { + // Calling getSignedInUserProfile() without a user logged in causes log + // noise that looks like an actual error... + fxAccounts.getSignedInUser().then(userData => { // userData may be null here when the user is not signed-in, but that's expected updateWithUserData(userData); - // unverified users cause us to spew log errors fetching an OAuth token - // to fetch the profile, so don't even try in that case. - if (!userData || !userData.verified || !profileInfoEnabled) { - return null; // don't even try to grab the profile. - } - return fxAccounts.getSignedInUserProfile().catch(err => { - // Not fetching the profile is sad but the FxA logs will already have noise. - return null; - }); + return userData ? fxAccounts.getSignedInUserProfile() : null; }).then(profile => { if (!profile) { return; @@ -335,7 +327,7 @@ let gFxAccounts = { // The most likely scenario is a user logged out, so reflect that. // Bug 995134 calls for better errors so we could retry if we were // sure this was the failure reason. - this.FxAccountsCommon.log.error("Error updating FxA account info", error); + this.FxAccountsCommon.log.error("Error updating FxA profile", error); updateWithUserData(null); }); }, diff --git a/browser/base/content/test/general/browser.ini b/browser/base/content/test/general/browser.ini index 4b19883871c..d0431cec30b 100644 --- a/browser/base/content/test/general/browser.ini +++ b/browser/base/content/test/general/browser.ini @@ -303,8 +303,6 @@ skip-if = true # browser_drag.js is disabled, as it needs to be updated for the [browser_focusonkeydown.js] [browser_fullscreen-window-open.js] skip-if = buildapp == 'mulet' || e10s || os == "linux" # Bug 933103 - mochitest's EventUtils.synthesizeMouse functions not e10s friendly. Linux: Intermittent failures - bug 941575. -[browser_fxaccounts.js] -support-files = fxa_profile_handler.sjs [browser_fxa_migrate.js] [browser_fxa_oauth.js] [browser_fxa_web_channel.js] diff --git a/browser/base/content/test/general/browser_fxaccounts.js b/browser/base/content/test/general/browser_fxaccounts.js deleted file mode 100644 index aef181860f6..00000000000 --- a/browser/base/content/test/general/browser_fxaccounts.js +++ /dev/null @@ -1,258 +0,0 @@ -/* Any copyright is dedicated to the Public Domain. - * http://creativecommons.org/publicdomain/zero/1.0/ */ - -let {Log} = Cu.import("resource://gre/modules/Log.jsm", {}); -let {Task} = Cu.import("resource://gre/modules/Task.jsm", {}); -let {fxAccounts} = Cu.import("resource://gre/modules/FxAccounts.jsm", {}); -let FxAccountsCommon = {}; -Cu.import("resource://gre/modules/FxAccountsCommon.js", FxAccountsCommon); - -const TEST_ROOT = "http://example.com/browser/browser/base/content/test/general/"; - -// instrument gFxAccounts to send observer notifications when it's done -// what it does. -(function() { - let unstubs = {}; // The original functions we stub out. - - // The stub functions. - let stubs = { - updateAppMenuItem: function() { - return unstubs['updateAppMenuItem'].call(gFxAccounts).then(() => { - Services.obs.notifyObservers(null, "test:browser_fxaccounts:updateAppMenuItem", null); - }); - }, - // Opening preferences is trickier than it should be as leaks are reported - // due to the promises it fires off at load time and there's no clear way to - // know when they are done. - // So just ensure openPreferences is called rather than whether it opens. - openPreferences: function() { - Services.obs.notifyObservers(null, "test:browser_fxaccounts:openPreferences", null); - } - }; - - for (let name in stubs) { - unstubs[name] = gFxAccounts[name]; - gFxAccounts[name] = stubs[name]; - } - // and undo our damage at the end. - registerCleanupFunction(() => { - for (let name in unstubs) { - gFxAccounts[name] = unstubs[name]; - } - stubs = unstubs = null; - }); -})(); - -// Other setup/cleanup -let newTab; - -Services.prefs.setCharPref("identity.fxaccounts.remote.signup.uri", - TEST_ROOT + "accounts_testRemoteCommands.html"); - -registerCleanupFunction(() => { - Services.prefs.clearUserPref("identity.fxaccounts.remote.signup.uri"); - Services.prefs.clearUserPref("identity.fxaccounts.remote.profile.uri"); - gBrowser.removeTab(newTab); -}); - -add_task(function* initialize() { - // Set a new tab with something other than about:blank, so it doesn't get reused. - // We must wait for it to load or the promiseTabOpen() call in the next test - // gets confused. - newTab = gBrowser.selectedTab = gBrowser.addTab("about:mozilla", {animate: false}); - yield promiseTabLoaded(newTab); -}); - -// The elements we care about. -let panelUILabel = document.getElementById("PanelUI-fxa-label"); -let panelUIStatus = document.getElementById("PanelUI-fxa-status"); -let panelUIFooter = document.getElementById("PanelUI-footer-fxa"); - -// The tests -add_task(function* test_nouser() { - let user = yield fxAccounts.getSignedInUser(); - Assert.strictEqual(user, null, "start with no user signed in"); - let promiseUpdateDone = promiseObserver("test:browser_fxaccounts:updateAppMenuItem"); - Services.obs.notifyObservers(null, this.FxAccountsCommon.ONLOGOUT_NOTIFICATION, null); - yield promiseUpdateDone; - - // Check the world - the FxA footer area is visible as it is offering a signin. - Assert.ok(isFooterVisible()) - - Assert.equal(panelUILabel.getAttribute("label"), panelUIStatus.getAttribute("defaultlabel")); - Assert.ok(!panelUIStatus.hasAttribute("tooltiptext"), "no tooltip when signed out"); - Assert.ok(!panelUIFooter.hasAttribute("fxastatus"), "no fxsstatus when signed out"); - Assert.ok(!panelUIFooter.hasAttribute("fxaprofileimage"), "no fxaprofileimage when signed out"); - - let promiseOpen = promiseTabOpen("about:accounts?entryPoint=menupanel"); - panelUIStatus.click(); - yield promiseOpen; -}); - -/* -XXX - Bug 1191162 - need a better hawk mock story or this will leak in debug builds. - -add_task(function* test_unverifiedUser() { - let promiseUpdateDone = promiseObserver("test:browser_fxaccounts:updateAppMenuItem"); - yield setSignedInUser(false); // this will fire the observer that does the update. - yield promiseUpdateDone; - - // Check the world. - Assert.ok(isFooterVisible()) - - Assert.equal(panelUILabel.getAttribute("label"), "foo@example.com"); - Assert.equal(panelUIStatus.getAttribute("tooltiptext"), - panelUIStatus.getAttribute("signedinTooltiptext")); - Assert.equal(panelUIFooter.getAttribute("fxastatus"), "signedin"); - let promisePreferencesOpened = promiseObserver("test:browser_fxaccounts:openPreferences"); - panelUIStatus.click(); - yield promisePreferencesOpened - yield signOut(); -}); -*/ - -add_task(function* test_verifiedUserEmptyProfile() { - // We see 2 updateAppMenuItem() calls - one for the signedInUser and one after - // we first fetch the profile. We want them both to fire or we aren't testing - // the state we think we are testing. - let promiseUpdateDone = promiseObserver("test:browser_fxaccounts:updateAppMenuItem", 2); - configureProfileURL({}); // successful but empty profile. - yield setSignedInUser(true); // this will fire the observer that does the update. - yield promiseUpdateDone; - - // Check the world. - Assert.ok(isFooterVisible()) - Assert.equal(panelUILabel.getAttribute("label"), "foo@example.com"); - Assert.equal(panelUIStatus.getAttribute("tooltiptext"), - panelUIStatus.getAttribute("signedinTooltiptext")); - Assert.equal(panelUIFooter.getAttribute("fxastatus"), "signedin"); - - let promisePreferencesOpened = promiseObserver("test:browser_fxaccounts:openPreferences"); - panelUIStatus.click(); - yield promisePreferencesOpened; - yield signOut(); -}); - -add_task(function* test_verifiedUserDisplayName() { - let promiseUpdateDone = promiseObserver("test:browser_fxaccounts:updateAppMenuItem", 2); - configureProfileURL({ displayName: "Test User Display Name" }); - yield setSignedInUser(true); // this will fire the observer that does the update. - yield promiseUpdateDone; - - Assert.ok(isFooterVisible()) - Assert.equal(panelUILabel.getAttribute("label"), "Test User Display Name"); - Assert.equal(panelUIStatus.getAttribute("tooltiptext"), - panelUIStatus.getAttribute("signedinTooltiptext")); - Assert.equal(panelUIFooter.getAttribute("fxastatus"), "signedin"); - yield signOut(); -}); - -add_task(function* test_verifiedUserProfileFailure() { - // profile failure means only one observer fires. - let promiseUpdateDone = promiseObserver("test:browser_fxaccounts:updateAppMenuItem", 1); - configureProfileURL(null, 500); - yield setSignedInUser(true); // this will fire the observer that does the update. - yield promiseUpdateDone; - - Assert.ok(isFooterVisible()) - Assert.equal(panelUILabel.getAttribute("label"), "foo@example.com"); - Assert.equal(panelUIStatus.getAttribute("tooltiptext"), - panelUIStatus.getAttribute("signedinTooltiptext")); - Assert.equal(panelUIFooter.getAttribute("fxastatus"), "signedin"); - yield signOut(); -}); - -// Helpers. -function isFooterVisible() { - let style = window.getComputedStyle(panelUIFooter); - return style.getPropertyValue("display") == "flex"; -} - -function configureProfileURL(profile, responseStatus = 200) { - let responseBody = profile ? JSON.stringify(profile) : ""; - let url = TEST_ROOT + "fxa_profile_handler.sjs?" + - "responseStatus=" + responseStatus + - "responseBody=" + responseBody + - // This is a bit cheeky - the FxA code will just append "/profile" - // to the preference value. We arrange for this to be seen by our - //.sjs as part of the query string. - "&path="; - - Services.prefs.setCharPref("identity.fxaccounts.remote.profile.uri", url); -} - -function promiseObserver(topic, count = 1) { - return new Promise(resolve => { - let obs = (subject, topic, data) => { - if (--count == 0) { - Services.obs.removeObserver(obs, topic); - resolve(subject); - } - } - Services.obs.addObserver(obs, topic, false); - }); -} - -// Stolen from browser_aboutHome.js -function promiseWaitForEvent(node, type, capturing) { - return new Promise((resolve) => { - node.addEventListener(type, function listener(event) { - node.removeEventListener(type, listener, capturing); - resolve(event); - }, capturing); - }); -} - -let promiseTabOpen = Task.async(function*(urlBase) { - info("Waiting for tab to open..."); - let event = yield promiseWaitForEvent(gBrowser.tabContainer, "TabOpen", true); - let tab = event.target; - yield promiseTabLoadEvent(tab); - ok(tab.linkedBrowser.currentURI.spec.startsWith(urlBase), - "Got " + tab.linkedBrowser.currentURI.spec + ", expecting " + urlBase); - let whenUnloaded = promiseTabUnloaded(tab); - gBrowser.removeTab(tab); - yield whenUnloaded; -}); - -function promiseTabUnloaded(tab) -{ - return new Promise(resolve => { - info("Wait for tab to unload"); - function handle(event) { - tab.linkedBrowser.removeEventListener("unload", handle, true); - info("Got unload event"); - resolve(event); - } - tab.linkedBrowser.addEventListener("unload", handle, true, true); - }); -} - -// FxAccounts helpers. -function setSignedInUser(verified) { - let data = { - email: "foo@example.com", - uid: "1234@lcip.org", - assertion: "foobar", - sessionToken: "dead", - kA: "beef", - kB: "cafe", - verified: verified, - - oauthTokens: { - // a token for the profile server. - profile: "key value", - } - } - return fxAccounts.setSignedInUser(data); -} - -let signOut = Task.async(function* () { - // This test needs to make sure that any updates for the logout have - // completed before starting the next test, or we see the observer - // notifications get out of sync. - let promiseUpdateDone = promiseObserver("test:browser_fxaccounts:updateAppMenuItem"); - // we always want a "localOnly" signout here... - yield fxAccounts.signOut(true); - yield promiseUpdateDone; -}); diff --git a/browser/base/content/test/general/fxa_profile_handler.sjs b/browser/base/content/test/general/fxa_profile_handler.sjs deleted file mode 100644 index 7160b76d0b2..00000000000 --- a/browser/base/content/test/general/fxa_profile_handler.sjs +++ /dev/null @@ -1,34 +0,0 @@ -/* Any copyright is dedicated to the Public Domain. - * http://creativecommons.org/publicdomain/zero/1.0/ */ - -// This is basically an echo server! -// We just grab responseStatus and responseBody query params! - -function reallyHandleRequest(request, response) { - var query = "?" + request.queryString; - - var responseStatus = 200; - var match = /responseStatus=([^&]*)/.exec(query); - if (match) { - responseStatus = parseInt(match[1]); - } - - var responseBody = ""; - match = /responseBody=([^&]*)/.exec(query); - if (match) { - responseBody = decodeURIComponent(match[1]); - } - - response.setStatusLine("1.0", responseStatus, "OK"); - response.write(responseBody); -} - -function handleRequest(request, response) -{ - try { - reallyHandleRequest(request, response); - } catch (e) { - response.setStatusLine("1.0", 500, "NotOK"); - response.write("Error handling request: " + e); - } -} diff --git a/browser/components/preferences/in-content/sync.js b/browser/components/preferences/in-content/sync.js index c4b4993f5c8..df4a677c6e0 100644 --- a/browser/components/preferences/in-content/sync.js +++ b/browser/components/preferences/in-content/sync.js @@ -119,7 +119,6 @@ let gSyncPane = { "weave:service:setup-complete", "weave:service:logout:finish", FxAccountsCommon.ONVERIFIED_NOTIFICATION, - FxAccountsCommon.ONLOGIN_NOTIFICATION, FxAccountsCommon.ON_PROFILE_CHANGE_NOTIFICATION, ]; let migrateTopic = "fxa-migration:state-changed"; From d86b53019e32cb08979335fe567fa2f6f4f13a11 Mon Sep 17 00:00:00 2001 From: Mark Hammond Date: Wed, 5 Aug 2015 15:50:36 +1000 Subject: [PATCH 28/28] Bug 1190279 - fix UI for unverified FxA users in both hamburger menu and Sync prefs pane. r=oeger --- .../content/aboutaccounts/aboutaccounts.js | 2 + browser/base/content/browser-fxaccounts.js | 22 +- browser/base/content/test/general/browser.ini | 2 + .../test/general/browser_fxaccounts.js | 258 ++++++++++++++++++ .../test/general/fxa_profile_handler.sjs | 34 +++ .../components/preferences/in-content/sync.js | 1 + 6 files changed, 312 insertions(+), 7 deletions(-) create mode 100644 browser/base/content/test/general/browser_fxaccounts.js create mode 100644 browser/base/content/test/general/fxa_profile_handler.sjs diff --git a/browser/base/content/aboutaccounts/aboutaccounts.js b/browser/base/content/aboutaccounts/aboutaccounts.js index 6fbfb9f3c1a..402635c34d9 100644 --- a/browser/base/content/aboutaccounts/aboutaccounts.js +++ b/browser/base/content/aboutaccounts/aboutaccounts.js @@ -358,6 +358,8 @@ function init() { } break; } + }).catch(err => { + error("Failed to get the signed in user: " + err); }); } diff --git a/browser/base/content/browser-fxaccounts.js b/browser/base/content/browser-fxaccounts.js index b45d6fb4801..f7f1092d80a 100644 --- a/browser/base/content/browser-fxaccounts.js +++ b/browser/base/content/browser-fxaccounts.js @@ -33,6 +33,7 @@ let gFxAccounts = { "weave:service:setup-complete", "weave:ui:login:error", "fxa-migration:state-changed", + this.FxAccountsCommon.ONLOGIN_NOTIFICATION, this.FxAccountsCommon.ONVERIFIED_NOTIFICATION, this.FxAccountsCommon.ONLOGOUT_NOTIFICATION, "weave:notification:removed", @@ -222,10 +223,11 @@ let gFxAccounts = { this.updateMigrationNotification(); }, + // Note that updateAppMenuItem() returns a Promise that's only used by tests. updateAppMenuItem: function () { if (this._migrationInfo) { this.updateAppMenuItemForMigration(); - return; + return Promise.resolve(); } let profileInfoEnabled = false; @@ -241,7 +243,7 @@ let gFxAccounts = { // state once migration is complete. this.panelUIFooter.hidden = true; this.panelUIFooter.removeAttribute("fxastatus"); - return; + return Promise.resolve(); } this.panelUIFooter.hidden = false; @@ -311,12 +313,18 @@ let gFxAccounts = { } } - // Calling getSignedInUserProfile() without a user logged in causes log - // noise that looks like an actual error... - fxAccounts.getSignedInUser().then(userData => { + return fxAccounts.getSignedInUser().then(userData => { // userData may be null here when the user is not signed-in, but that's expected updateWithUserData(userData); - return userData ? fxAccounts.getSignedInUserProfile() : null; + // unverified users cause us to spew log errors fetching an OAuth token + // to fetch the profile, so don't even try in that case. + if (!userData || !userData.verified || !profileInfoEnabled) { + return null; // don't even try to grab the profile. + } + return fxAccounts.getSignedInUserProfile().catch(err => { + // Not fetching the profile is sad but the FxA logs will already have noise. + return null; + }); }).then(profile => { if (!profile) { return; @@ -327,7 +335,7 @@ let gFxAccounts = { // The most likely scenario is a user logged out, so reflect that. // Bug 995134 calls for better errors so we could retry if we were // sure this was the failure reason. - this.FxAccountsCommon.log.error("Error updating FxA profile", error); + this.FxAccountsCommon.log.error("Error updating FxA account info", error); updateWithUserData(null); }); }, diff --git a/browser/base/content/test/general/browser.ini b/browser/base/content/test/general/browser.ini index d0431cec30b..4b19883871c 100644 --- a/browser/base/content/test/general/browser.ini +++ b/browser/base/content/test/general/browser.ini @@ -303,6 +303,8 @@ skip-if = true # browser_drag.js is disabled, as it needs to be updated for the [browser_focusonkeydown.js] [browser_fullscreen-window-open.js] skip-if = buildapp == 'mulet' || e10s || os == "linux" # Bug 933103 - mochitest's EventUtils.synthesizeMouse functions not e10s friendly. Linux: Intermittent failures - bug 941575. +[browser_fxaccounts.js] +support-files = fxa_profile_handler.sjs [browser_fxa_migrate.js] [browser_fxa_oauth.js] [browser_fxa_web_channel.js] diff --git a/browser/base/content/test/general/browser_fxaccounts.js b/browser/base/content/test/general/browser_fxaccounts.js new file mode 100644 index 00000000000..aef181860f6 --- /dev/null +++ b/browser/base/content/test/general/browser_fxaccounts.js @@ -0,0 +1,258 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ */ + +let {Log} = Cu.import("resource://gre/modules/Log.jsm", {}); +let {Task} = Cu.import("resource://gre/modules/Task.jsm", {}); +let {fxAccounts} = Cu.import("resource://gre/modules/FxAccounts.jsm", {}); +let FxAccountsCommon = {}; +Cu.import("resource://gre/modules/FxAccountsCommon.js", FxAccountsCommon); + +const TEST_ROOT = "http://example.com/browser/browser/base/content/test/general/"; + +// instrument gFxAccounts to send observer notifications when it's done +// what it does. +(function() { + let unstubs = {}; // The original functions we stub out. + + // The stub functions. + let stubs = { + updateAppMenuItem: function() { + return unstubs['updateAppMenuItem'].call(gFxAccounts).then(() => { + Services.obs.notifyObservers(null, "test:browser_fxaccounts:updateAppMenuItem", null); + }); + }, + // Opening preferences is trickier than it should be as leaks are reported + // due to the promises it fires off at load time and there's no clear way to + // know when they are done. + // So just ensure openPreferences is called rather than whether it opens. + openPreferences: function() { + Services.obs.notifyObservers(null, "test:browser_fxaccounts:openPreferences", null); + } + }; + + for (let name in stubs) { + unstubs[name] = gFxAccounts[name]; + gFxAccounts[name] = stubs[name]; + } + // and undo our damage at the end. + registerCleanupFunction(() => { + for (let name in unstubs) { + gFxAccounts[name] = unstubs[name]; + } + stubs = unstubs = null; + }); +})(); + +// Other setup/cleanup +let newTab; + +Services.prefs.setCharPref("identity.fxaccounts.remote.signup.uri", + TEST_ROOT + "accounts_testRemoteCommands.html"); + +registerCleanupFunction(() => { + Services.prefs.clearUserPref("identity.fxaccounts.remote.signup.uri"); + Services.prefs.clearUserPref("identity.fxaccounts.remote.profile.uri"); + gBrowser.removeTab(newTab); +}); + +add_task(function* initialize() { + // Set a new tab with something other than about:blank, so it doesn't get reused. + // We must wait for it to load or the promiseTabOpen() call in the next test + // gets confused. + newTab = gBrowser.selectedTab = gBrowser.addTab("about:mozilla", {animate: false}); + yield promiseTabLoaded(newTab); +}); + +// The elements we care about. +let panelUILabel = document.getElementById("PanelUI-fxa-label"); +let panelUIStatus = document.getElementById("PanelUI-fxa-status"); +let panelUIFooter = document.getElementById("PanelUI-footer-fxa"); + +// The tests +add_task(function* test_nouser() { + let user = yield fxAccounts.getSignedInUser(); + Assert.strictEqual(user, null, "start with no user signed in"); + let promiseUpdateDone = promiseObserver("test:browser_fxaccounts:updateAppMenuItem"); + Services.obs.notifyObservers(null, this.FxAccountsCommon.ONLOGOUT_NOTIFICATION, null); + yield promiseUpdateDone; + + // Check the world - the FxA footer area is visible as it is offering a signin. + Assert.ok(isFooterVisible()) + + Assert.equal(panelUILabel.getAttribute("label"), panelUIStatus.getAttribute("defaultlabel")); + Assert.ok(!panelUIStatus.hasAttribute("tooltiptext"), "no tooltip when signed out"); + Assert.ok(!panelUIFooter.hasAttribute("fxastatus"), "no fxsstatus when signed out"); + Assert.ok(!panelUIFooter.hasAttribute("fxaprofileimage"), "no fxaprofileimage when signed out"); + + let promiseOpen = promiseTabOpen("about:accounts?entryPoint=menupanel"); + panelUIStatus.click(); + yield promiseOpen; +}); + +/* +XXX - Bug 1191162 - need a better hawk mock story or this will leak in debug builds. + +add_task(function* test_unverifiedUser() { + let promiseUpdateDone = promiseObserver("test:browser_fxaccounts:updateAppMenuItem"); + yield setSignedInUser(false); // this will fire the observer that does the update. + yield promiseUpdateDone; + + // Check the world. + Assert.ok(isFooterVisible()) + + Assert.equal(panelUILabel.getAttribute("label"), "foo@example.com"); + Assert.equal(panelUIStatus.getAttribute("tooltiptext"), + panelUIStatus.getAttribute("signedinTooltiptext")); + Assert.equal(panelUIFooter.getAttribute("fxastatus"), "signedin"); + let promisePreferencesOpened = promiseObserver("test:browser_fxaccounts:openPreferences"); + panelUIStatus.click(); + yield promisePreferencesOpened + yield signOut(); +}); +*/ + +add_task(function* test_verifiedUserEmptyProfile() { + // We see 2 updateAppMenuItem() calls - one for the signedInUser and one after + // we first fetch the profile. We want them both to fire or we aren't testing + // the state we think we are testing. + let promiseUpdateDone = promiseObserver("test:browser_fxaccounts:updateAppMenuItem", 2); + configureProfileURL({}); // successful but empty profile. + yield setSignedInUser(true); // this will fire the observer that does the update. + yield promiseUpdateDone; + + // Check the world. + Assert.ok(isFooterVisible()) + Assert.equal(panelUILabel.getAttribute("label"), "foo@example.com"); + Assert.equal(panelUIStatus.getAttribute("tooltiptext"), + panelUIStatus.getAttribute("signedinTooltiptext")); + Assert.equal(panelUIFooter.getAttribute("fxastatus"), "signedin"); + + let promisePreferencesOpened = promiseObserver("test:browser_fxaccounts:openPreferences"); + panelUIStatus.click(); + yield promisePreferencesOpened; + yield signOut(); +}); + +add_task(function* test_verifiedUserDisplayName() { + let promiseUpdateDone = promiseObserver("test:browser_fxaccounts:updateAppMenuItem", 2); + configureProfileURL({ displayName: "Test User Display Name" }); + yield setSignedInUser(true); // this will fire the observer that does the update. + yield promiseUpdateDone; + + Assert.ok(isFooterVisible()) + Assert.equal(panelUILabel.getAttribute("label"), "Test User Display Name"); + Assert.equal(panelUIStatus.getAttribute("tooltiptext"), + panelUIStatus.getAttribute("signedinTooltiptext")); + Assert.equal(panelUIFooter.getAttribute("fxastatus"), "signedin"); + yield signOut(); +}); + +add_task(function* test_verifiedUserProfileFailure() { + // profile failure means only one observer fires. + let promiseUpdateDone = promiseObserver("test:browser_fxaccounts:updateAppMenuItem", 1); + configureProfileURL(null, 500); + yield setSignedInUser(true); // this will fire the observer that does the update. + yield promiseUpdateDone; + + Assert.ok(isFooterVisible()) + Assert.equal(panelUILabel.getAttribute("label"), "foo@example.com"); + Assert.equal(panelUIStatus.getAttribute("tooltiptext"), + panelUIStatus.getAttribute("signedinTooltiptext")); + Assert.equal(panelUIFooter.getAttribute("fxastatus"), "signedin"); + yield signOut(); +}); + +// Helpers. +function isFooterVisible() { + let style = window.getComputedStyle(panelUIFooter); + return style.getPropertyValue("display") == "flex"; +} + +function configureProfileURL(profile, responseStatus = 200) { + let responseBody = profile ? JSON.stringify(profile) : ""; + let url = TEST_ROOT + "fxa_profile_handler.sjs?" + + "responseStatus=" + responseStatus + + "responseBody=" + responseBody + + // This is a bit cheeky - the FxA code will just append "/profile" + // to the preference value. We arrange for this to be seen by our + //.sjs as part of the query string. + "&path="; + + Services.prefs.setCharPref("identity.fxaccounts.remote.profile.uri", url); +} + +function promiseObserver(topic, count = 1) { + return new Promise(resolve => { + let obs = (subject, topic, data) => { + if (--count == 0) { + Services.obs.removeObserver(obs, topic); + resolve(subject); + } + } + Services.obs.addObserver(obs, topic, false); + }); +} + +// Stolen from browser_aboutHome.js +function promiseWaitForEvent(node, type, capturing) { + return new Promise((resolve) => { + node.addEventListener(type, function listener(event) { + node.removeEventListener(type, listener, capturing); + resolve(event); + }, capturing); + }); +} + +let promiseTabOpen = Task.async(function*(urlBase) { + info("Waiting for tab to open..."); + let event = yield promiseWaitForEvent(gBrowser.tabContainer, "TabOpen", true); + let tab = event.target; + yield promiseTabLoadEvent(tab); + ok(tab.linkedBrowser.currentURI.spec.startsWith(urlBase), + "Got " + tab.linkedBrowser.currentURI.spec + ", expecting " + urlBase); + let whenUnloaded = promiseTabUnloaded(tab); + gBrowser.removeTab(tab); + yield whenUnloaded; +}); + +function promiseTabUnloaded(tab) +{ + return new Promise(resolve => { + info("Wait for tab to unload"); + function handle(event) { + tab.linkedBrowser.removeEventListener("unload", handle, true); + info("Got unload event"); + resolve(event); + } + tab.linkedBrowser.addEventListener("unload", handle, true, true); + }); +} + +// FxAccounts helpers. +function setSignedInUser(verified) { + let data = { + email: "foo@example.com", + uid: "1234@lcip.org", + assertion: "foobar", + sessionToken: "dead", + kA: "beef", + kB: "cafe", + verified: verified, + + oauthTokens: { + // a token for the profile server. + profile: "key value", + } + } + return fxAccounts.setSignedInUser(data); +} + +let signOut = Task.async(function* () { + // This test needs to make sure that any updates for the logout have + // completed before starting the next test, or we see the observer + // notifications get out of sync. + let promiseUpdateDone = promiseObserver("test:browser_fxaccounts:updateAppMenuItem"); + // we always want a "localOnly" signout here... + yield fxAccounts.signOut(true); + yield promiseUpdateDone; +}); diff --git a/browser/base/content/test/general/fxa_profile_handler.sjs b/browser/base/content/test/general/fxa_profile_handler.sjs new file mode 100644 index 00000000000..7160b76d0b2 --- /dev/null +++ b/browser/base/content/test/general/fxa_profile_handler.sjs @@ -0,0 +1,34 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ */ + +// This is basically an echo server! +// We just grab responseStatus and responseBody query params! + +function reallyHandleRequest(request, response) { + var query = "?" + request.queryString; + + var responseStatus = 200; + var match = /responseStatus=([^&]*)/.exec(query); + if (match) { + responseStatus = parseInt(match[1]); + } + + var responseBody = ""; + match = /responseBody=([^&]*)/.exec(query); + if (match) { + responseBody = decodeURIComponent(match[1]); + } + + response.setStatusLine("1.0", responseStatus, "OK"); + response.write(responseBody); +} + +function handleRequest(request, response) +{ + try { + reallyHandleRequest(request, response); + } catch (e) { + response.setStatusLine("1.0", 500, "NotOK"); + response.write("Error handling request: " + e); + } +} diff --git a/browser/components/preferences/in-content/sync.js b/browser/components/preferences/in-content/sync.js index df4a677c6e0..c4b4993f5c8 100644 --- a/browser/components/preferences/in-content/sync.js +++ b/browser/components/preferences/in-content/sync.js @@ -119,6 +119,7 @@ let gSyncPane = { "weave:service:setup-complete", "weave:service:logout:finish", FxAccountsCommon.ONVERIFIED_NOTIFICATION, + FxAccountsCommon.ONLOGIN_NOTIFICATION, FxAccountsCommon.ON_PROFILE_CHANGE_NOTIFICATION, ]; let migrateTopic = "fxa-migration:state-changed";