Closed Bug 1239848 Opened 4 years ago Closed 4 years ago

Backout about:profiles (bug 1232629) until it's ready to ship

Categories

(Toolkit :: Startup and Profile System, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox46 --- fixed

People

(Reporter: Dolske, Assigned: baku)

References

Details

Attachments

(1 file)

There are a lot of issues with the new profile manager (about:profiles), and it's not ready to ship in Firefox 46. We should back this out before uplift on the 26th, and instead target the 47 train.

UX also feels that the current implementation needs enough work that it should be disabled on Nightly until it's in an improved state. (See 1232629 comment 4 -- although I'm not sure if a pref will work here, given that prefs are stored in a profile. So might need an ongoing backout on Nightly too. Or maybe a build flag?)
Andrea, is this something you can do next week?
Flags: needinfo?(amarchesini)
Yes, working on it.
Flags: needinfo?(amarchesini)
Assignee: nobody → amarchesini
Attached patch revert.patchSplinter Review
Ehsan, I'm working with Bram to propose something better from the UX point of view.
Attachment #8709384 - Flags: review?(ehsan)
Comment on attachment 8709384 [details] [diff] [review]
revert.patch

Review of attachment 8709384 [details] [diff] [review]:
-----------------------------------------------------------------

Sigh...  I'm not thrilled about going all the way back to the old UI again, but I guess this is fine.  :(

You may want to send an email to the l10n mailing list letting localizers know about this change, in case our tools can't figure out the disappearing and reappearing strings.

Also, for the next iteration, I would like to find someone who will own the decision on whether the thing that we have is "good enough" before we land it initially to avoid having to do this again...  That person needs to r+ any patch that relands the new UI, and then I can r- any further attempts to rip it out.  ;-)
Attachment #8709384 - Flags: review?(ehsan) → review+
(In reply to :Ehsan Akhgari from comment #4)

> Also, for the next iteration, I would like to find someone who will own the
> decision on whether the thing that we have is "good enough" before we land
> it initially to avoid having to do this again...

That would be Philipp Sackl. I think things will work out ok as long as we've got a plan with UX for what to implement, and keep an eye out for unexpected bugs.
(In reply to Justin Dolske [:Dolske] from comment #5)
> (In reply to :Ehsan Akhgari from comment #4)
> 
> > Also, for the next iteration, I would like to find someone who will own the
> > decision on whether the thing that we have is "good enough" before we land
> > it initially to avoid having to do this again...
> 
> That would be Philipp Sackl.

Awesome!

> I think things will work out ok as long as
> we've got a plan with UX for what to implement, and keep an eye out for
> unexpected bugs.

Yeah, I agree.  Looks like Andrea and Bram are discussing that.

Normally we'd implement new UIs behind a pref to eliminate this pain, but the profile manager needs to work without a profile, and therefore without access to prefs...  Oh well!
Yeah I'm happy to do that.
I just looked over Brams UI work that he is doing together with Andrea and it looks really promising.
Duplicate of this bug: 1234496
Duplicate of this bug: 1235332
Duplicate of this bug: 1233072
Duplicate of this bug: 1233022
Duplicate of this bug: 1236846
Duplicate of this bug: 1237520
Duplicate of this bug: 1236867
(In reply to :Ehsan Akhgari from comment #4)
> You may want to send an email to the l10n mailing list letting localizers
> know about this change, in case our tools can't figure out the disappearing
> and reappearing strings.

Most of our localization teams work against mozilla-aurora, so luckily they won't see any change if this back-out lands before merge day. Still worth a message to dev-l10n though, explaining that this concerns only Nightly.

The annoying part is actually the removal of strings for the old profile manager, that now need to be added back
http://hg.mozilla.org/mozilla-central/rev/a2705f50d061

I guess it's not safe to keep the new strings around, because the new profile manager (v2) might be a lot different?
(In reply to Francesco Lodolo [:flod] from comment #16)
> (In reply to :Ehsan Akhgari from comment #4)
> > You may want to send an email to the l10n mailing list letting localizers
> > know about this change, in case our tools can't figure out the disappearing
> > and reappearing strings.
> 
> Most of our localization teams work against mozilla-aurora, so luckily they
> won't see any change if this back-out lands before merge day. Still worth a
> message to dev-l10n though, explaining that this concerns only Nightly.

Good to know, thanks!  Needinfoing baku so that he doesn't forget.  :-)

> The annoying part is actually the removal of strings for the old profile
> manager, that now need to be added back
> http://hg.mozilla.org/mozilla-central/rev/a2705f50d061

Yes.  That's one of the reasons why I didn't like this patch.

> I guess it's not safe to keep the new strings around, because the new
> profile manager (v2) might be a lot different?

It's hard to say, but honestly I prefer to remove them now and ask people to ignore the churn and don't waste their time potentially re-translating slightly different strings...
Flags: needinfo?(amarchesini)
https://hg.mozilla.org/mozilla-central/rev/1a29c77b2600
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Note: Bug 1240660 removed support for "+" prefixes in jar manifests. You need to update the following two lines:
https://hg.mozilla.org/mozilla-central/rev/1a29c77b2600#l7.12
https://hg.mozilla.org/mozilla-central/rev/1a29c77b2600#l9.12
(In reply to :Ehsan Akhgari from comment #17)
> It's hard to say, but honestly I prefer to remove them now and ask people to
> ignore the churn and don't waste their time potentially re-translating
> slightly different strings...

But we didn't, did we? I've just realized that this patch removes one string from aboutProfiles.properties but leaves the two aboutProfiles.* in tree. 

We should remove them given the discussion: people will see strings in Aurora and have no idea what they're about (nor be able to test them).
Depends on: 1241608
(In reply to Philip Chee from comment #19)
> Note: Bug 1240660 removed support for "+" prefixes in jar manifests. You
> need to update the following two lines:
> https://hg.mozilla.org/mozilla-central/rev/1a29c77b2600#l7.12
> https://hg.mozilla.org/mozilla-central/rev/1a29c77b2600#l9.12

I don't understand what I have to do. There is not such '+' prefix in these 2 lines.
Flags: needinfo?(amarchesini) → needinfo?(philip.chee)
Blocks: 1241893
No longer depends on: 1241608
Duplicate of this bug: 1233008
Duplicate of this bug: 1234497
Duplicate of this bug: 1234765
Duplicate of this bug: 1237518
(In reply to Francesco Lodolo [:flod] from comment #20)
> But we didn't, did we? I've just realized that this patch removes one string
> from aboutProfiles.properties but leaves the two aboutProfiles.* in tree. 
> 
> We should remove them given the discussion: people will see strings in
> Aurora and have no idea what they're about (nor be able to test them).

Reopening NI. If we want to remove the strings, this *need* to happen before the merge to Aurora, which is Monday.
Flags: needinfo?(amarchesini)
> I don't understand what I have to do. There is not such '+' prefix in these
> 2 lines.
Oops sorry
Flags: needinfo?(philip.chee)
Duplicate of this bug: 1235832
Duplicate of this bug: 1235295
Duplicate of this bug: 1235284
And removing NI, since this ship has sailed :-\
Flags: needinfo?(amarchesini)
I see that about:profiles works on the latest version, but I don't see any revisions to the corresponding mozilla-central files since https://hg.mozilla.org/mozilla-central/rev/1a29c77b2600 , so I assume it's still not ready. Is there a bug tracking implementation of the new about:profiles?
Bug 1240022 is about implementing a new profile manager.
You need to log in before you can comment on or make changes to this bug.