Last Comment Bug 689832 - syncSetup.js refers to outdated string change.password.pwSameAsSyncKey
: syncSetup.js refers to outdated string change.password.pwSameAsSyncKey
Status: RESOLVED FIXED
[qa?]
:
Product: Cloud Services
Classification: Client Software
Component: Firefox Sync: UI (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla10
Assigned To: Allison Naaktgeboren :ally
:
:
Mentors:
Depends on:
Blocks: 656492
  Show dependency treegraph
 
Reported: 2011-09-27 19:34 PDT by Philipp von Weitershausen [:philikon]
Modified: 2011-12-19 04:44 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed


Attachments
Part 1 (v0) Fix missing renamed key (1.01 KB, patch)
2011-09-28 11:40 PDT, Allison Naaktgeboren :ally
philipp: review+
asa: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Philipp von Weitershausen [:philikon] 2011-09-27 19:34:06 PDT
(From bug 656492 comment 29)
> AFAICS you guys missed one change.password.pwSameAsSyncKey occurrence in
> syncSetup.js (I'll leave that one to you since I need to hurry and port this
> one to SM in time for the Aurora uplift).
Comment 1 Allison Naaktgeboren :ally 2011-09-28 11:40:19 PDT
Created attachment 563127 [details] [diff] [review]
Part 1 (v0) Fix missing renamed key
Comment 2 Philipp von Weitershausen [:philikon] 2011-09-28 12:14:38 PDT
Comment on attachment 563127 [details] [diff] [review]
Part 1 (v0) Fix missing renamed key

r=me. Ally, please land this on m-i (s-c is closed right now for the QA train.)

Requesting approval for Aurora as this fixes as a reference to a no longer existent string that we missed in bug 656492.
Comment 3 Allison Naaktgeboren :ally 2011-09-28 12:52:04 PDT
Landed in inbound:

  http://hg.mozilla.org/integration/mozilla-inbound/rev/6da4c8939412
Comment 4 Michael Wu [:mwu] 2011-09-29 01:33:24 PDT
https://hg.mozilla.org/mozilla-central/rev/6da4c8939412
Comment 5 Philipp von Weitershausen [:philikon] 2011-09-29 15:21:15 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/2de758311c15
Comment 6 Mihaela Velimiroviciu (:mihaelav) 2011-11-14 07:12:32 PST
While verifying this bug, I notice that the change involves some validation on password when it is changed: as far as I understood, it should trigger an error message when password is changed to the recovery key(please correct me if I'm wrong), but it actually doesn't (you can change the password to be the same as the recovery key). Is this expected?

Build ids:
Mozilla/5.0 (Windows NT 6.1; rv:9.0) Gecko/20100101 Firefox/9.0
Mozilla/5.0 (Windows NT 6.1; rv:10.0a2) Gecko/20111113 Firefox/10.0a2
Mozilla/5.0 (Windows NT 6.1; rv:11.0a1) Gecko/20111113 Firefox/11.0a1
Comment 7 Alex Keybl [:akeybl] 2011-12-13 16:11:04 PST
(In reply to Mihaela Velimiroviciu [QA] from comment #6)
> While verifying this bug, I notice that the change involves some validation
> on password when it is changed: as far as I understood, it should trigger an
> error message when password is changed to the recovery key(please correct me
> if I'm wrong), but it actually doesn't (you can change the password to be
> the same as the recovery key). Is this expected?

[Triage Comment]
Would somebody mind addressing Mihaela's comment by tomorrow at 12:00PM PT 12/14? We will be holding our FF9 sign-offs later that afternoon, and need to be able to verify. Thanks!
Comment 8 Philipp von Weitershausen [:philikon] 2011-12-13 21:10:55 PST
(In reply to Alex Keybl [:akeybl] from comment #7)
> (In reply to Mihaela Velimiroviciu [QA] from comment #6)
> > While verifying this bug, I notice that the change involves some validation
> > on password when it is changed: as far as I understood, it should trigger an
> > error message when password is changed to the recovery key(please correct me
> > if I'm wrong), but it actually doesn't (you can change the password to be
> > the same as the recovery key). Is this expected?
> 
> [Triage Comment]
> Would somebody mind addressing Mihaela's comment by tomorrow at 12:00PM PT
> 12/14? We will be holding our FF9 sign-offs later that afternoon, and need
> to be able to verify. Thanks!

Please please file a new bug for things like these. Commenting on RESOLVED/FIXED bugs will get very little attention. Also, when reporting a problem, please try to include exact STRs (comment 6 does not give them) and a regression range, if a regression is suspected. (Tbh, these are things I would expect from QA when they find a bug, without asking for them.)

I have gone ahead and looked closer at the issue. It seems like we haven't removed this check that Mihaela is talking about, just the error message string that would appear. In any case, I have filed bug 710543 for this. Mihaela, can you please put your STRs and other findings there, please?

As far as the FF9 sign-off is concerned, I personally would consider this issue a total non-blocker. Now that users no longer choose the Sync Key themselves in the form of a passphrase, they would actively have to copy & paste the Sync Key into the password change field. That's a pretty rare edge case.
Comment 9 Richard Newman [:rnewman] 2011-12-13 21:53:40 PST
(In reply to Philipp von Weitershausen [:philikon] from comment #8)

> Please please file a new bug for things like these. Commenting on
> RESOLVED/FIXED bugs will get very little attention.

I can't second this enough. If you're concerned that something might be enough to hold a release, a quiet "is this expected?" in a closed bug is insufficient.

Given this vague comment on a closed non-critical bug, I was leaving this in my "to take a look at early next year" queue. A new bug, an expression of urgency, thorough steps to reproduce, and a regression window will all help to shift a question or report from "that'll take time and isn't important, so I'll look at it when I'm less slammed" into "I can address this now", so please help us to help you.

> In any case, I have filed bug 710543 for this.

Thanks, Philipp.

> As far as the FF9 sign-off is concerned, I personally would consider this
> issue a total non-blocker.

Agreed.
Comment 10 Alex Keybl [:akeybl] 2011-12-14 08:25:50 PST
(In reply to Philipp von Weitershausen [:philikon] from comment #8)
> Please please file a new bug for things like these. Commenting on
> RESOLVED/FIXED bugs will get very little attention.

I just checked my bugmail settings and there's nothing to suggest that a comment in a  resolved/fixed bug would not show up in my mailbox. Is that not true for others?


(In reply to Richard Newman [:rnewman] from comment #9)
> Given this vague comment on a closed non-critical bug, I was leaving this in
> my "to take a look at early next year" queue. A new bug, an expression of
> urgency, thorough steps to reproduce, and a regression window will all help
> to shift a question or report from "that'll take time and isn't important,
> so I'll look at it when I'm less slammed" into "I can address this now", so
> please help us to help you.

It's unclear why you this is now considered non-critical? This bug was taken on Aurora 9 for (presumably) that very reason. If it's critical enough to uplift into a branch later than mozilla-central, it's critical enough to address any concerns that QA may have or ask for clarification prior to the release date. Putting off a question here is not the correct next action.
Comment 11 Philipp von Weitershausen [:philikon] 2011-12-14 12:18:37 PST
(In reply to Alex Keybl [:akeybl] from comment #10)
> (In reply to Philipp von Weitershausen [:philikon] from comment #8)
> > Please please file a new bug for things like these. Commenting on
> > RESOLVED/FIXED bugs will get very little attention.
> 
> I just checked my bugmail settings and there's nothing to suggest that a
> comment in a  resolved/fixed bug would not show up in my mailbox. Is that
> not true for others?

Oh, it's true. It's just that I get a lot of bugmail, and my in-brain bugmail filtering works very similar to how rnewman described it. A "btw" comment on a RESO/FIXED bug triggers different synapses than a new bug marked as "regression".

> (In reply to Richard Newman [:rnewman] from comment #9)
> > Given this vague comment on a closed non-critical bug, I was leaving this in
> > my "to take a look at early next year" queue. A new bug, an expression of
> > urgency, thorough steps to reproduce, and a regression window will all help
> > to shift a question or report from "that'll take time and isn't important,
> > so I'll look at it when I'm less slammed" into "I can address this now", so
> > please help us to help you.
> 
> It's unclear why you this is now considered non-critical? This bug was taken
> on Aurora 9 for (presumably) that very reason.

Sure. This is different in many ways. First off, bug 675823, the cause of the regression, was landed in Firefox 10. So I don't see how FF9 is affected. Once again, Mihaela's report is unclear here, so unless she can confirm a problem on FF9, I'm going to assume that only FF10+ are affected.

Secondly, changing code to refer to the right string ID is different from changing strings. Strings are normally frozen when stuff goes to Aurora. I haven't dealt with this scenario before, but I will investigate. All the more reason to do this in a new bug!

> If it's critical enough to
> uplift into a branch later than mozilla-central, it's critical enough to
> address any concerns that QA may have or ask for clarification prior to the
> release date. Putting off a question here is not the correct next action.

I believe the correct action -- correct in terms of our engineering process -- is to file a bug, determine what causes the regression, which versions of Firefox are affected, produce a fix, and then discuss the risks of backporting said fix to any affected branches, should there be any besides mozilla-central. Do you not agree?
Comment 12 Alex Keybl [:akeybl] 2011-12-16 13:18:20 PST
(In reply to Philipp von Weitershausen [:philikon] from comment #11)
> Sure. This is different in many ways. First off, bug 675823, the cause of
> the regression, was landed in Firefox 10. So I don't see how FF9 is
> affected. Once again, Mihaela's report is unclear here, so unless she can
> confirm a problem on FF9, I'm going to assume that only FF10+ are affected.

I'm not sure what you mean here. My point is that when QA asked a question to help them verify this uplifted bug, they did not receive an answer. I just want to make sure that QA gets what they need to verify our bug fixes.

> I believe the correct action -- correct in terms of our engineering process
> -- is to file a bug, determine what causes the regression, which versions of
> Firefox are affected, produce a fix, and then discuss the risks of
> backporting said fix to any affected branches, should there be any besides
> mozilla-central. Do you not agree?

I do agree that this is the correct next step for QA to do in the future, but we need to make sure that we help out when that process isn't followed exactly.
Comment 13 Philipp von Weitershausen [:philikon] 2011-12-19 04:44:01 PST
(In reply to Alex Keybl [:akeybl] from comment #12)
> (In reply to Philipp von Weitershausen [:philikon] from comment #11)
> > Sure. This is different in many ways. First off, bug 675823, the cause of
> > the regression, was landed in Firefox 10. So I don't see how FF9 is
> > affected. Once again, Mihaela's report is unclear here, so unless she can
> > confirm a problem on FF9, I'm going to assume that only FF10+ are affected.
> 
> I'm not sure what you mean here. My point is that when QA asked a question
> to help them verify this uplifted bug, they did not receive an answer. I
> just want to make sure that QA gets what they need to verify our bug fixes.

Understood. In this case the answer is: unrelated to this bug and unrelated to Firefox 9.

Note You need to log in before you can comment on or make changes to this bug.