One of the common doorhangers that users get. We should clean up the type, spacing, etc.. around this

We use the same visuals for all of our doorhanger notifications, so this bug should really be about updating those shared styles.

Yes, i definitely want to share the styles as much as possible. I think it's because the original meta was more about visuals so I focused this on a specific one to track work.

I've been iterating on these for a while and took heavy inspiration from a lot of the work Stephen's been doing around these things for the Desktop side. I've both adapted those conventions for mobile, and taken into account all of the latest UI changes/ new features we've been working on. For symmetry and clarity, I've cleaned up our doorhangers styles here and aimed to maintain the defining qualities of our old ones too specifically around spoof-ability and context (where is this thing coming from?). This work will be pretty important as a lot of the Passwords UX stuff, tracking protection work, etc happens in the doorhanger.

Tracking ID mock

site ID mock

The canonical source for the mocks is now: https://www.lucidchart.com/documents/edit/87ab1cc8-e708-49d3-8b91-6e2e6da346fb/1

Bug 1129619 is the Desktop version of this.

/r/5303 - Bug 1088220 - WIP Split Doorhanger to allow different Doorhanger types. Pull down this commit: hg pull review -r 4cdf5747814bcfc79e7fb5d61df7661b63504926

Next: non-editable save/update password doorhangers.

Comment on attachment 8577038 [details] MozReview Request: bz://1088220/liuche /r/5303 - Bug 1088220 - Split DoorHanger to support other doorhangers. r=margaret /r/5413 - Bug 1088220 - Rename DoorHanger to Doorhanger. r=margaret Pull down these commits: hg pull review -r bbd3712579079c5d60f16c97b0acc39470314be2

https://reviewboard.mozilla.org/r/5413/#review4367 Reviewboard sucks at hg rename, so it's hard to follow this, but as long as this is a staight-up variable reame, rubber stamp from me.

https://reviewboard.mozilla.org/r/5303/#review4369 ::: mobile/android/base/resources/layout/site_identity.xml (Diff revision 2) > + <LinearLayout android:layout_width="match_parent" Adding another nested LinearLayout feels a bit smelly, but probably the bigger issue is the existing view hierarchy we have in here. Something to consider cleaning up in the future if/when we change the site identity appearance (e.g. getting rid of larry). ::: mobile/android/base/resources/layout/site_identity.xml (Diff revision 2) > + <View android:id="@+id/divider_doorhanger" Adding a divider to SiteIdentityPopup feels a bit out of the scope of this bug, but it doesn't look too complicated, so I'll allow it :) ::: mobile/android/base/widget/DefaultDoorHanger.java (Diff revision 2) > + public static enum Theme { It looks like we're getting rid of the dark theme, so you can just get rid of this enum. ::: mobile/android/base/widget/DefaultDoorHanger.java (Diff revision 2) > + super(context, tabId, id, Type.DEFAULT); Are you missing something here? Where is Type defined? I think you may have accidentally omitted changes to DoorHanger.java from this patch.

This is a bit of a nightmare - it looks like reviewboard doesn't handle renames where you re-create a file that was renamed, so I'm just uploading this as a patch. This includes the DoorHanger.java changes. I'm leaving off the rename patch for now, because for some reason, mercurial is also having a case collision folding error when making only case-changes to file names (even though OSX is a case-sensitive OS!), and it's destroying files that have been committed by deleting them. I don't want to deal with this right now, so I'll change it in a follow-up bug or something. > ::: mobile/android/base/resources/layout/site_identity.xml > (Diff revision 2) > > + <LinearLayout android:layout_width="match_parent" > > Adding another nested LinearLayout feels a bit smelly, but probably the > bigger issue is the existing view hierarchy we have in here. Something to > consider cleaning up in the future if/when we change the site identity > appearance (e.g. getting rid of larry). Agreed - this is so that we can add the divider in SiteIdentityPopup, because Anthony says we can remove the dark theme, which greatly simplifies abstracting out the Doorhanger code (which is used in both SiteIdentityPopup and DoorHangerPopup). When we turn SiteIdentityPopup into a Real Popup that inherits from DoorHanger, we can avoid this, but right now SiteIdentityPopup is hardcoded to load the site_identity layout xml. > ::: mobile/android/base/widget/DefaultDoorHanger.java > (Diff revision 2) > > + super(context, tabId, id, Type.DEFAULT); > > Are you missing something here? Where is Type defined? I think you may have > accidentally omitted changes to DoorHanger.java from this patch. This should be present in this patch, although it wasn't showing up on reviewboard for some reason...

Comment on attachment 8578325 [details] [diff] [review] Patch: Split Doorhanger. Review of attachment 8578325 [details] [diff] [review]: ----------------------------------------------------------------- This is a bit hard to review because there are lots of changes in DefaultDoorHanger.java and DoorHanger.java. I think it might have been easier if the split was done in one patch, and then the logic/renaming changes done in a second. Please make sure you test this with complicated doorhangers, like the WebRTC one. This is looking good, but I'd like to see a new version and the answers to some questions before giving an r+. ::: mobile/android/base/DoorHangerPopup.java @@ +228,5 @@ > * Gets a doorhanger. > * > * This method must be called on the UI thread. > */ > DoorHanger getDoorHanger(int tabId, String value) { You could file a follow-up bug to update this parameter to be `identifier` instead. ::: mobile/android/base/widget/DefaultDoorHanger.java @@ -310,5 @@ > - Rect rect = new Rect(); > - spinner.getBackground().getPadding(rect); > - > - // Set the difference in padding to the spinner view to align it with doorhanger text. > - view.setPadding(sInputPadding - rect.left - textPadding, 0, rect.right, sInputPadding); Why is it okay to get rid of all this logic? ::: mobile/android/base/widget/DoorHanger.java @@ +13,5 @@ > import android.text.style.ForegroundColorSpan; > import android.text.style.URLSpan; > import android.view.LayoutInflater; > import android.view.View; > +import android.widget.*; Please replace this with specific class imports. @@ +55,2 @@ > > + // TODO: pull these from JSON message. This doesn't sound like a TODO... we already do this below. @@ -162,5 @@ > } > > - public void setMessage(String message) { > - Spanned markupMessage = Html.fromHtml(message); > - mTextView.setMovementMethod(LinkMovementMethod.getInstance()); // Necessary for clickable links Why did you drop this line? @@ +173,5 @@ > public void onClick(View v) { > listener.onButtonClick(DoorHanger.this, tag); > } > }); > + mButtonsContainer.setVisibility(View.VISIBLE); Looks like you accidentally copied this line here when you didn't mean to. @@ -211,2 @@ > divider.setVisibility(View.VISIBLE); > - divider.setBackgroundColor(mDividerColor); Why can we remove this?

(In reply to :Margaret Leibovic from comment #16) > Comment on attachment 8578325 [details] [diff] [review] > Patch: Split Doorhanger. > > Review of attachment 8578325 [details] [diff] [review]: > ----------------------------------------------------------------- > > This is a bit hard to review because there are lots of changes in > DefaultDoorHanger.java and DoorHanger.java. I think it might have been > easier if the split was done in one patch, and then the logic/renaming > changes done in a second. Please make sure you test this with complicated > doorhangers, like the WebRTC one. > > This is looking good, but I'd like to see a new version and the answers to > some questions before giving an r+. > > ::: mobile/android/base/DoorHangerPopup.java > @@ +228,5 @@ > > * Gets a doorhanger. > > * > > * This method must be called on the UI thread. > > */ > > DoorHanger getDoorHanger(int tabId, String value) { > > You could file a follow-up bug to update this parameter to be `identifier` > instead. Okay, I might just change this in the later patches for unifying siteIdentity - variable renaming seems so trivial x_x > > ::: mobile/android/base/widget/DefaultDoorHanger.java > @@ -310,5 @@ > > - Rect rect = new Rect(); > > - spinner.getBackground().getPadding(rect); > > - > > - // Set the difference in padding to the spinner view to align it with doorhanger text. > > - view.setPadding(sInputPadding - rect.left - textPadding, 0, rect.right, sInputPadding); > > Why is it okay to get rid of all this logic? I was looking at why all this logic was here in the first place, and it's apparently there to shift the dropdown option menus closer to the edge of the doorhanger so that the text all lines up. This isn't how the UI is intended to work (the edge of the dropdown menu should be aligned with the line of text, not the text of the dropdown menu) - it's a lot of complexity to hack the default intended Android UI. Talked this over with antlam, he says we should use the default behavior. > @@ -162,5 @@ > > } > > > > - public void setMessage(String message) { > > - Spanned markupMessage = Html.fromHtml(message); > > - mTextView.setMovementMethod(LinkMovementMethod.getInstance()); // Necessary for clickable links > > Why did you drop this line? There is a method for adding a link that sets the MovementMethod, and I didn't find any doorhangers that had links in the actual text. > @@ -211,2 @@ > > divider.setVisibility(View.VISIBLE); > > - divider.setBackgroundColor(mDividerColor); > > Why can we remove this? Since we don't use the dark theme anymore (I checked with Anthony), this can be set through xml instead dynamically.

Removed TODOs that were unnecessary, updated imports, removed extra mButtonsContainer line that was unnecessary.

Dammit, I didn't realize that IntelliJ was autocombining my imports for >5 imports from a single component. Updated this in the patch.

(In reply to Chenxia Liu [:liuche] from comment #17) > > @@ -162,5 @@ > > > } > > > > > > - public void setMessage(String message) { > > > - Spanned markupMessage = Html.fromHtml(message); > > > - mTextView.setMovementMethod(LinkMovementMethod.getInstance()); // Necessary for clickable links > > > > Why did you drop this line? > > There is a method for adding a link that sets the MovementMethod, and I > didn't find any doorhangers that had links in the actual text. This was added in bug 964412 for add-ons to use, so I think we should keep it here.

Comment on attachment 8577038 [details] MozReview Request: bz://1088220/liuche /r/5303 - Bug 1088220 - Split DoorHanger to support other doorhangers. r=margaret /r/5413 - Bug 1088220 - Add key icon resources. r=margaret /r/5923 - Bug 1088220 - Add Config for Doorhangers. r=margaret /r/5925 - Bug 1088220 - Switch to using DoorhangerConfig. r=margaret /r/5927 - Bug 1088220 - Add styling changes for new Login doorhanger. r=margaret /r/5929 - Bug 1088220 - Add login doorhanger. r=margaret Pull down these commits: hg pull review -r f3be63111aefcbc7fa5fecf1e9ac6d5129898c2f

Margaret: I know you've already reviewed the first commit in this queue via a patch, but reviewboard is just confused.

Comment on attachment 8582191 [details] Screenshot: Doorhangers This is looking great! few notes: - we still need to center align the doorhangers - "allimoz" should be in our Link blue? (or is it just not pressable right now?) - Affirmative actions should be consistently on the right, while negative actions on the left (swap around Update and Don't update) - Can we try to give Affirmative action buttons the blue background (white text)? - In the same line, could we give negative action buttons the Toolbar menu dark grey background color? - can we remove the orange divider too? Thanks chenxia!

Cool, I'll update the dividers to #D7D9D8 and add it to the palette as menu divider grey, and I'll swap the buttons. > > - we still need to center align the doorhangers bug 1139551 > - "allimoz" should be in our Link blue? (or is it just not pressable right > now?) bug 1144385 > - Can we try to give Affirmative action buttons the blue background (white > text)? > - In the same line, could we give negative action buttons the Toolbar menu > dark grey background color? bug 1147064, I'll update all the doorhangers at once, since this is just for the login doorhanger.

(In reply to Chenxia Liu [:liuche] from comment #24) > Cool, I'll update the dividers to #D7D9D8 I meant #D7D9DB...

Comment on attachment 8577038 [details] MozReview Request: bz://1088220/liuche /r/5303 - Bug 1088220 - Split DoorHanger to support other doorhangers. r=margaret /r/5413 - Bug 1088220 - Add key icon resources. r=margaret /r/5923 - Bug 1088220 - Add Config for Doorhangers. r=margaret /r/5925 - Bug 1088220 - Switch to using DoorhangerConfig. r=margaret /r/5927 - Bug 1088220 - Add styling changes for new Login doorhanger. r=margaret /r/5929 - Bug 1088220 - Add login doorhanger. r=margaret /r/5985 - Bug 1088220 - Update text size for doorhangers. r=margaret Pull down these commits: hg pull review -r 07125a25ffaf371f1a6d36229a79a6689e92dfb9

Addressed Anthony's comments and fixed doorhanger text sizing.

https://reviewboard.mozilla.org/r/5413/#review4939 Ship It!

https://reviewboard.mozilla.org/r/5923/#review4941 ::: mobile/android/base/widget/DoorhangerConfig.java (Diff revision 2) > +package org.mozilla.gecko.widget; Add a license header. ::: mobile/android/base/widget/DoorhangerConfig.java (Diff revision 2) > +public class DoorhangerConfig { I feel like we should call this DoorHangerConfig to be consistent with the current DoorHanger objects... but I really don't care that much, since things will just not build if you mess it up :) ::: mobile/android/base/widget/DoorhangerConfig.java (Diff revision 2) > + public void addTabId(int tabId) { These methods should probably be called `setFoo` not `addFoo`, since they replace any existing thing. Also, there's no constructor for this class? I haven't looked at the future patches where you start using this yet, but it seems odd that we wouldn't use a constructor to initialize this with all its properties, especially ones that I imagaine shouldn't ever change, like `id`.

https://reviewboard.mozilla.org/r/5925/#review4943 ::: mobile/android/base/DoorHangerPopup.java (Diff revision 2) > - void addDoorHanger(final int tabId, final String value, final String message, > + void addDoorHanger(DoorhangerConfig config) { This is definitely a lot nicer than the long argument list. ::: mobile/android/base/DoorHangerPopup.java (Diff revision 2) > + final DoorhangerConfig config = new DoorhangerConfig(); I feel like we should be passing the non-optional properties into a DoorhangerConfig constructor here, and then make them immutable. Right now it feels a bit dangerous that anyone who has their hands on a Doorhanger config object could go around changing its state. ::: mobile/android/base/toolbar/SiteIdentityPopup.java (Diff revision 2) > - message = mContext.getString(R.string.loaded_mixed_content_message); > + config.addMessage(mContext.getString(R.string.loaded_mixed_content_message)); Yeah, definitely think these should be `setMessage` not `addMessage`, since right now it looks like it's possible to have a list of messages. ::: mobile/android/base/widget/DoorHanger.java (Diff revision 2) > + public static DoorHanger Get(Context context, DoorhangerConfig config) { Why `Get` with a captial G?

https://reviewboard.mozilla.org/r/5929/#review4949 ::: mobile/android/base/widget/DoorHanger.java (Diff revision 2) > case PASSWORD: Should we call this LOGIN to be consistent? ::: mobile/android/components/LoginManagerPrompter.js (Diff revision 2) > get _strBundle() { Let's get a mentor bug on file to fix the indentation in this file :) ::: mobile/android/components/LoginManagerPrompter.js (Diff revision 2) > - _showLoginNotification : function (aName, aText, aButtons) { > + _showLoginNotification : function (aName, aTitle, aBody, aButtons, aSubtext) { We should update the documentation here to explain what these parameters are. ::: mobile/android/chrome/content/browser.js (Diff revision 2) > - show: function(aMessage, aValue, aButtons, aTabID, aOptions) { > + show: function(aMessage, aValue, aButtons, aTabID, aOptions, aTitle, aSubtext, aCategory) { I think we should make title/subtext/category part of the options object to avoid having such a long parameter list here. ::: mobile/android/components/LoginManagerPrompter.js (Diff revision 2) > + var displayHost = this._getShortDisplayHost(aOldLogin.hostname); Unless you have a compelling reason not to, you should use `let`, not `var`. We could also get a mentor bug to un-var-ify this file. ::: mobile/locales/en-US/overrides/passwordmgr.properties (Diff revision 2) > -# String is the login's hostname > +neverButton=Never You may need to update strings in the robocop tests.

https://reviewboard.mozilla.org/r/5985/#review4951 ::: mobile/android/base/resources/values/styles.xml (Diff revision 1) > - <style name="TextAppearance.Widget.DoorHanger.Small.Light"/> > + <style name="DoorHangerTextAppearance.Medium.Light"/> How does this work? Shouldn't this have parent="DoorHangerTestAppearance.Medium"? If we just set a textAppearence to this, will it have the correct font size?

https://reviewboard.mozilla.org/r/5923/#review4959 > I feel like we should call this DoorHangerConfig to be consistent with the current DoorHanger objects... but I really don't care that much, since things will just not build if you mess it up :) I'm pretty meh about this, and I want to make everything Doorhanger anyways. Filed bug 1147201. Also, I don't want to go through and rename everything now and then rename it back in that bug... > These methods should probably be called `setFoo` not `addFoo`, since they replace any existing thing. > > Also, there's no constructor for this class? I haven't looked at the future patches where you start using this yet, but it seems odd that we wouldn't use a constructor to initialize this with all its properties, especially ones that I imagaine shouldn't ever change, like `id`. Good point - I made a constructor for tabId and id and made them final fields. We still need an empty constructor for SiteIdentityPopup because it doesn't care what tab it's in because it's added/removed on every show/dismissal, so these aren't technically required fields. But that will change in the future! so we can just remove the empty constructor then.

https://reviewboard.mozilla.org/r/5925/#review4961 > Why `Get` with a captial G? It's Java convention to use a capitalized Get for static getter methods in this not-really-Factory-but-kinda pattern.

https://reviewboard.mozilla.org/r/5929/#review4957 > Unless you have a compelling reason not to, you should use `let`, not `var`. > > We could also get a mentor bug to un-var-ify this file. Filed bug 1147211. I'll make it mentorable once we don't need to touch this file as much. > Let's get a mentor bug on file to fix the indentation in this file :) Filed bug 1147197. > I think we should make title/subtext/category part of the options object to avoid having such a long parameter list here. I kept category as a top-level argument because we need that to select which doorhanger to create (and this will be useful in the future when we break DefaultDoorHanger out into separate doorhangers). > You may need to update strings in the robocop tests. Yep, writing a patch for fixing the tests as a separate commit.

https://reviewboard.mozilla.org/r/5985/#review4953 > How does this work? Shouldn't this have parent="DoorHangerTestAppearance.Medium"? If we just set a textAppearence to this, will it have the correct font size? The dot hierarchy in Android styles implies a parent relationship; the reason the old code here explicitly set parent was because the dot hierarchy didn't line up (in fact, our style files mix the parenting hierarchy and some homespun naming hierarchy that is very un-Android). So DHTA.Medium and DHTA.Small both automatically inherit the characteristics set in DoorHangerTextAppearance. If you try to use a dot hierarchy using an implicit parent that doesn't actually exist, the build will actually fail and complain that a parent can't be found.

Comment on attachment 8577038 [details] MozReview Request: bz://1088220/liuche /r/5303 - Bug 1088220 - Split DoorHanger to support other doorhangers. r=margaret /r/5413 - Bug 1088220 - Add key icon resources. r=margaret /r/5923 - Bug 1088220 - Add Config for Doorhangers. r=margaret /r/5925 - Bug 1088220 - Switch to using DoorhangerConfig. r=margaret /r/5927 - Bug 1088220 - Add styling changes for new Login doorhanger. r=margaret /r/6109 - Bug 1088220 - Add login doorhanger. r=margaret /r/6111 - Bug 1088220 - Style changes. r=margaret /r/6117 - Bug 1088220 - Update tests with new strings. r=margaret Pull down these commits: hg pull review -r f596d2b8641d785c54c20bfdad69d449abf05bb9

Try run for the updated tests (ran them locally first already): https://treeherder.mozilla.org/#/jobs?repo=try&revision=0c578181c64d

Switching back to Nightly, I realized that with the new mocks, we're swapping the confirm/dismiss buttons on the doorhanger, and also turning the dismiss into "Never". I wonder if people will accidentally muscle-memory the wrong button (if they tend to hit "Not now") all the time. ...an undo toast could be nice.

(In reply to Chenxia Liu [:liuche] from comment #41) > Switching back to Nightly, I realized that with the new mocks, we're > swapping the confirm/dismiss buttons on the doorhanger, and also turning the > dismiss into "Never". I wonder if people will accidentally muscle-memory the > wrong button (if they tend to hit "Not now") all the time. > > ...an undo toast could be nice. antlam, how do you feel about these points? Users can undo the "never" action by editing site settings, but that's not really discoverable right now. I think the undo toast could be a good idea, maybe we should file a follow-up for that.

https://reviewboard.mozilla.org/r/6109/#review5127 ::: mobile/android/base/DoorHangerPopup.java (Diff revision 1) > - final int tabId = json.getInt("tabId"); > + final int tabId = json.getInt("tabID"); This change should really be in the earlier commit where you introduced this, but if you fold these all together to land, that won't matter :) ::: mobile/android/base/widget/LoginDoorHanger.java (Diff revision 1) > + } catch (JSONException e) { The try/catch should probably just be around the `getString` calls that will throw. ::: mobile/android/base/widget/LoginDoorHanger.java (Diff revision 1) > + final String url = titleObj.getString("resource"); If `resource` is indeed optional, this should be `optString`. I would also name this local variable `resource` to be consistent (or alternately name the JSON property `url`). ::: mobile/android/chrome/content/browser.js (Diff revision 1) > + * I think you should add some more details about the format of this resource URL, since it's not clear what's supported. Also, it might reduce confusion to rename the `title` property in here to something like `message`, since it's already part of a `title` property. ::: mobile/android/components/LoginManagerPrompter.js (Diff revision 1) > + * String to be displayed below the aBody message Nice documentation update. ::: mobile/android/components/LoginManagerPrompter.js (Diff revision 1) > - } else { > + let subtext = this._sanitizeUsername(aLogin.username); Looks like you'll run into a JS error if aLogin.username is null/undefined: http://mxr.mozilla.org/mozilla-central/source/mobile/android/components/LoginManagerPrompter.js#394 I think we should keep the truth-y check around this (or update _sanitizeUsername).

Switching the confirm/dismiss was really a move to unify these interactions to be more consistent with both themselves (sometimes we're inconsistent) as well as system (on mobile, often right is confirm, left is dismiss). But the intent was originally to keep consistent with Desktop copy ("Never" and "Remember"). Also, since tapping anywhere outside dismisses the doorhanger, it seemed alright. However, we can definitely revisit this later if we think it's weird on a mobile platform. Hm, I'm open to the idea of a confirmation toast with an undo action. Something like "Login saved with Firefox | Undo" ?

https://reviewboard.mozilla.org/r/5923/#review5129 Ship It!

https://reviewboard.mozilla.org/r/5925/#review5131 Ship It!

https://reviewboard.mozilla.org/r/5927/#review5133 Ship It!

https://reviewboard.mozilla.org/r/6109/#review5135 r+ with issues addressed.

https://reviewboard.mozilla.org/r/6111/#review5137 I would say this should be combined with the other style changes commit, but I don't care enough that you should change that now :)

https://reviewboard.mozilla.org/r/6117/#review5139 ::: mobile/android/base/tests/StringHelper.java (Diff revision 1) > + public static final String CONTEXT_MENU_SITE_SETTINGS_SAVE_PASSWORD = "Save Password"; We'll have some other bug to make sure our use of "passowrds" and "logins" is consistent, right?

Comment on attachment 8577038 [details] MozReview Request: bz://1088220/liuche https://reviewboard.mozilla.org/r/5301/#review5141 Ship It!

https://reviewboard.mozilla.org/r/6109/#review5179 > I think you should add some more details about the format of this resource URL, since it's not clear what's supported. > > Also, it might reduce confusion to rename the `title` property in here to something like `message`, since it's already part of a `title` property. I wanted to leave this a little open-ended because this might be handled differently by different doorhangers, but for now I made it more specific and added a note about generalizing.

https://reviewboard.mozilla.org/r/6117/#review5177 > We'll have some other bug to make sure our use of "passowrds" and "logins" is consistent, right? Bug 1136477

Comment on attachment 8577038 [details] MozReview Request: bz://1088220/liuche /r/5303 - Bug 1088220 - Split DoorHanger to support other doorhangers. r=margaret /r/5413 - Bug 1088220 - Add key icon resources. r=margaret /r/5923 - Bug 1088220 - Add Config for Doorhangers. r=margaret /r/5925 - Bug 1088220 - Switch to using DoorhangerConfig. r=margaret /r/5927 - Bug 1088220 - Add login doorhanger. r=margaret /r/6109 - Bug 1088220 - Update tests with new strings. r=margaret Pull down these commits: hg pull review -r 9a15d2d5ae25e12c50b379f30b99ecab42820c9e

Comment on attachment 8577038 [details] MozReview Request: bz://1088220/liuche Carrying over r+.

https://hg.mozilla.org/integration/fx-team/rev/30d0f12f958c https://hg.mozilla.org/integration/fx-team/rev/3249fc039d08 https://hg.mozilla.org/integration/fx-team/rev/c8a569d73bb0 https://hg.mozilla.org/integration/fx-team/rev/e734718125b0 https://hg.mozilla.org/integration/fx-team/rev/ebf592c2fa4c https://hg.mozilla.org/integration/fx-team/rev/28bc7e898980

https://hg.mozilla.org/mozilla-central/rev/30d0f12f958c https://hg.mozilla.org/mozilla-central/rev/3249fc039d08 https://hg.mozilla.org/mozilla-central/rev/c8a569d73bb0 https://hg.mozilla.org/mozilla-central/rev/e734718125b0 https://hg.mozilla.org/mozilla-central/rev/ebf592c2fa4c https://hg.mozilla.org/mozilla-central/rev/28bc7e898980

Do these new patches still need to land in Nightly?

Sorry about this bugspam - I'm not sure at all what this is from, but there are no new patches.

Liz - this is Resolved, Fixed. :)