1331800 - Removing imported history / bookmarks / passwords gets "stuck" in some cases, leading to a confusing user experience

Status: RESOLVED FIXED

(Firefox :: Migration, defect)

Description

[Justin [:JW_SoftvisionQA]]

Mozilla/5.0 (Windows NT 6.3; WOW64; rv:51.0) Gecko/20100101 Firefox/51.0

see video below:

https://jwilliams-softvision.tinytake.com/sf/MTI3MzY4M180Nzc2OTEy

As you can see it takes about 1 minute for my tabs, browser history, etc. to be automigrated over from Chrome, which freezes my browser. Once it is automigrated, Opening about:newtab does produce the dont keep option. On Win 7 and 10 the dont keep button works as intended but on win 8.1 it only deletes the bookmarks. The browser history and history tiles stay. Opening about:newtab again produces the same prompt "We automatically imported your data from Google Chrome. Would you like to keep it?". This prompt shows over and over on win 8.1.

Browser Console shows:

Could not read chrome manifest 'file:///C:/Program%20Files%20(x86)/Mozilla%20Firefox/chrome.manifest'.
can't access lexical declaration `tab' before initialization  TelemetryReportingPolicy.jsm:440
	_openFirstRunPage/progressListener.onStateChange resource://gre/modules/TelemetryReportingPolicy.jsm:440:1
	callListeners chrome://browser/content/tabbrowser.xml:490:24
	_callProgressListeners chrome://browser/content/tabbrowser.xml:511:13
	mTabProgressListener/<._callProgressListeners chrome://browser/content/tabbrowser.xml:560:22
	mTabProgressListener/<.onStateChange chrome://browser/content/tabbrowser.xml:729:15
	_loadURIWithFlags chrome://browser/content/browser.js:807:7
	loadURIWithFlags chrome://browser/content/tabbrowser.xml:7064:13
	addTab chrome://browser/content/tabbrowser.xml:2110:17
	loadOneTab chrome://browser/content/tabbrowser.xml:1523:23
	_openFirstRunPage resource://gre/modules/TelemetryReportingPolicy.jsm:462:15
	TelemetryReportingPolicyImpl.observe resource://gre/modules/TelemetryReportingPolicy.jsm:478:13
	initializeWindow resource:///modules/sessionstore/SessionStore.jsm:1050:9
	SessionStoreInternal.onBeforeBrowserWindowShown/< resource:///modules/sessionstore/SessionStore.jsm:1200:9
	Handler.prototype.process resource://gre/modules/Promise-backend.js:937:23
	this.PromiseWalker.walkerLoop resource://gre/modules/Promise-backend.js:816:7
	bound  self-hosted
	bound bound  self-hosted
	this.PromiseWalker.scheduleWalkerLoop/< resource://gre/modules/Promise-backend.js:750:11
Content Security Policy: Directive ‘frame-src’ has been deprecated. Please use directive ‘child-src’ instead. (unknown)
Key event not available on some keyboard layouts: key=“r” modifiers=“accel,alt” id=“toggleReaderMode”  browser.xul
Key event not available on some keyboard layouts: key=“i” modifiers=“accel,alt,shift” id=“key_browserToolbox”  browser.xul
Login data scheme type not supported: 3 ChromeProfileMigrator.js:477
	GetWindowsPasswordsResource/<.migrate< resource://app/components/ChromeProfileMigrator.js:477:21
	next self-hosted
	TaskImpl_run resource://gre/modules/Task.jsm:319:40
	bound TaskImpl_run self-hosted
	Handler.prototype.process resource://gre/modules/Promise-backend.js:937:23
	this.PromiseWalker.walkerLoop resource://gre/modules/Promise-backend.js:816:7
	bound  self-hosted
	bound bound  self-hosted
	this.PromiseWalker.scheduleWalkerLoop/< resource://gre/modules/Promise-backend.js:750:11
The page was reloaded, because the character encoding declaration of the HTML document was not found when prescanning the first 1024 bytes of the file. The encoding declaration needs to be moved to be within the first 1024 bytes of the file.  juice.asp:8
A promise chain failed to handle a rejection. Did you forget to '.catch', or did you forget to 'return'?
See https://developer.mozilla.org/Mozilla/JavaScript_code_modules/Promise.jsm/Promise

Date: Tue Jan 17 2017 15:54:13 GMT-0800 (Pacific Standard Time)
Full Message: Error: page-thumbnail:error
Full Stack: observe@resource://gre/modules/BackgroundPageThumbs.jsm:116:22
BackgroundPageThumbs._onCaptureOrTimeout@resource://gre/modules/BackgroundPageThumbs.jsm:305:7
Capture.prototype._done/done@resource://gre/modules/BackgroundPageThumbs.jsm:451:7
Capture.prototype._done@resource://gre/modules/BackgroundPageThumbs.jsm:471:7
Capture.prototype.notify@resource://gre/modules/BackgroundPageThumbs.jsm:428:5

Comment 1

[:Gijs (he/him)]

(In reply to Justin [:JW_SoftvisionQA] from comment #0)
> Mozilla/5.0 (Windows NT 6.3; WOW64; rv:51.0) Gecko/20100101 Firefox/51.0
> 
> see video below:
> 
> https://jwilliams-softvision.tinytake.com/sf/MTI3MzY4M180Nzc2OTEy
> 
> As you can see it takes about 1 minute for my tabs, browser history, etc. to
> be automigrated over from Chrome, which freezes my browser.

Yes, this is a known issue. See bug 1289217. I expect there is quite a lot of history in the chrome profile...

> Once it is
> automigrated, Opening about:newtab does produce the dont keep option. On Win
> 7 and 10 the dont keep button works as intended but on win 8.1 it only
> deletes the bookmarks. The browser history and history tiles stay. Opening
> about:newtab again produces the same prompt "We automatically imported your
> data from Google Chrome. Would you like to keep it?". This prompt shows over
> and over on win 8.1.

So, none of the errors listed in your comment are related to the undo... Does the history maybe get cleared if you give it 5 (or 10, or...) minutes? The fact that the notification bar keeps appearing is because the undo doesn't complete. I'll file a separate bug for that (to not show the notification again while the undo is ongoing), but what I'm most curious about is whether the process does actually eventually complete, and/or where it's gotten stuck...

If you open the library (ctrl-shift-b) and switch to the history view, do you see anything being removed after clicking the "don't keep" button?

As a vague indicator of how much stuff is being removed, how big (in terms of filesize) is the 'initialMigrationMetadata.jsonlz4' file in your profile directory?

Finally, if you go into about:telemetry and check the FX_STARTUP_MIGRATION_AUTOMATED_IMPORT_UNDO histogram, what does that look like?

Comment 2

[:Gijs (he/him)]

(In reply to :Gijs from bug 1331888 comment #17)
> (In reply to Justin [:JW_SoftvisionQA] from bug 1331888 comment #15)
> > Just a [note] Gijs, I do not think that the undo is taking a lot of time
> > because of the size of the history. I used the same google chrome profile on
> > win 7, 8.1, & 10. Win 7 & 10 did it in milliseconds. Win 8.1 did not do it
> > at all.
> 
> That's surprising. There isn't really OS-specific code involved here, so I
> don't know why the behaviour would be different on Windows 8. Let's look at
> this in bug 1331800.

Justin, can I have a look at this over Vidyo?

Comment 3

[Justin [:JW_SoftvisionQA]]

yes. Give me about 5 minutes to set everything up.

Comment 4

[Justin [:JW_SoftvisionQA]]

Im going to your Vidyo room now Gijs.

Comment 5

[:Gijs (he/him)]

Attachment: Bug 1331800 - catch errors from history removals and don't block undo on them,

Review commit: https://reviewboard.mozilla.org/r/105560/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/105560/

Comment 6

[Justin Dolske [:Dolske]]

(In reply to Justin [:JW_SoftvisionQA] from comment #0)

> Login data scheme type not supported: 3 ChromeProfileMigrator.js:477

Hmm. A quick indicates https://chromium.googlesource.com/chromium/src/+/master/components/autofill/core/common/password_form.h#53 is probably (?) the definition of this field, which makes "3" just the vague "SCHEME_OTHER" (ftp, maybe?). And there's also a type 4 for USERNAME_ONLY, which we don't really support in our password manager.

So feels like one bug here is to see what "3" is, and if it's something we should be importing. Justin: any idea what login in Chrome is triggering this from your profile? (If not, we could spin a build with some extra debugging or give you some SQL to check manually...)

Feels like another bug is that if this is one of the items throwing in the Undo code, we should fix that to not save logins IDs that didn't actually get saved. (Although IIRC you modified addLogin to do just that, so maybe this isn't happening?)

Thirdly, I think I saw comments elsewhere that from debugging to saw this throwing when undoing History in Justin's VM? Do we know what was actually happening and why?

I think the addition of the try/catch here is still a good safety measure, but would still be useful to understand what's actually going wrong.

Comment 7

[Justin [:JW_SoftvisionQA]]

Hey Dolske, Gijs was able to send me a build that worked flawlessly. Do you still want me to look into this issue?

Comment 8

[:Gijs (he/him)]

(In reply to Justin Dolske [:Dolske] from comment #6)
> (In reply to Justin [:JW_SoftvisionQA] from comment #0)
> 
> > Login data scheme type not supported: 3 ChromeProfileMigrator.js:477
> 
> Hmm. A quick indicates
> https://chromium.googlesource.com/chromium/src/+/master/components/autofill/
> core/common/password_form.h#53 is probably (?) the definition of this field,
> which makes "3" just the vague "SCHEME_OTHER" (ftp, maybe?). And there's
> also a type 4 for USERNAME_ONLY, which we don't really support in our
> password manager.

So, some clarification: this reported error is an error from the import, which works fine pre-patch. The issue is with undo-ing the import. That issue (ie where we get stuck) is also with history, and this error is from password import.

I did also add error catching to the password undo bits because in theory there could be similar issues there, I guess. In general, I think I've had a change of heart about how this UI should work. I used to think that it would make sense to have errors break the 'undo' so you could try again (ie we wouldn't remove the data files and so on so you could click the button again and maybe it'd work next time). I think in practice that's a rubbish user experience, and pretending it worked even if it broke halfway through is more likely to be a "usable" experience - if things are really in the way, users can delete them themselves, but they have effectively no recourse to being continuously nagged to do something (ie use the 'undo' functionality) that doesn't work.

> So feels like one bug here is to see what "3" is, and if it's something we
> should be importing. Justin: any idea what login in Chrome is triggering
> this from your profile? (If not, we could spin a build with some extra
> debugging or give you some SQL to check manually...)
> 
> Feels like another bug is that if this is one of the items throwing in the
> Undo code, we should fix that to not save logins IDs that didn't actually
> get saved. (Although IIRC you modified addLogin to do just that, so maybe
> this isn't happening?)

Yeah, that isn't happening with logins. We would only add 'maybe undo this' info if the login actually got added/modified.

It's hard to do the same for history because we batch-add everything and don't really have ids for individual visits, AIUI.

> Thirdly, I think I saw comments elsewhere that from debugging to saw this
> throwing when undoing History in Justin's VM? Do we know what was actually
> happening and why?

So, we didn't see it actually throwing, but we saw it starting to import and not finishing. We also tried to use the debugger's "break on uncaught exceptions", and that didn't break on anything relevant, unfortunately.

I tried locally, and if you yield for a rejected promise in the _removeSomeVisits code in AutoMigrate.jsm, ie:

yield new Promise((resolve, reject) => reject());

the same symptoms happen when running the automated test (ie it just hangs indefinitely without reporting any errors). I believe that's a bug - AIUI Task.spawn/Task.async is supposed to reject/throw, and eventually that rejection/exception should be showing up in the error console, but it seems like that doesn't happen without some 'help' here. Anyway, wasn't going to go down that rabbithole for 51.

> I think the addition of the try/catch here is still a good safety measure,
> but would still be useful to understand what's actually going wrong.

Yeah. I added Cu.reportError 'logging' for that purpose. One thing that confuses me about this is that Justin reported he's not seeing any errors in the browser console with the build I sent (which is based off nightly) - it just works, whereas the previous build did not. I'm worried that this might be caused by differences between beta and nightly. I did a diff of the migration directory and didn't turn up anything. To be safe, here's a try push with the same patch based off beta (which I hope builds on try without further modifications):

remote:   https://treeherder.mozilla.org/#/jobs?repo=try&revision=5dd777cdac27e75256f17398efbe1220622670b9

Justin W., can you clarify if using the "Don't keep" option with builds from this trypush (you'll still need to put in the funnelcake distribution dir manually) works? And there's really nothing that gets logged to the console?

Comment 9

[Justin [:JW_SoftvisionQA]]

The win 8. build works as intended. View the contents of the Browser Console Below:


          Login data scheme type not supported: 3 ChromeProfileMigrator.js:477
	GetWindowsPasswordsResource/<.migrate< resource://app/components/ChromeProfileMigrator.js:477:21
	next self-hosted
	TaskImpl_run resource://gre/modules/Task.jsm:319:40
	bound TaskImpl_run self-hosted
	Handler.prototype.process resource://gre/modules/Promise-backend.js:937:23
	this.PromiseWalker.walkerLoop resource://gre/modules/Promise-backend.js:816:7
	bound  self-hosted
	bound bound  self-hosted
	this.PromiseWalker.scheduleWalkerLoop/< resource://gre/modules/Promise-backend.js:750:11
The page was reloaded, because the character encoding declaration of the HTML document was not found when prescanning the first 1024 bytes of the file. The encoding declaration needs to be moved to be within the first 1024 bytes of the file.  juice.asp:8
A promise chain failed to handle a rejection. Did you forget to '.catch', or did you forget to 'return'?
See https://developer.mozilla.org/Mozilla/JavaScript_code_modules/Promise.jsm/Promise

Date: Wed Jan 18 2017 15:16:38 GMT-0800 (Pacific Standard Time)
Full Message: Error: page-thumbnail:error
Full Stack: observe@resource://gre/modules/BackgroundPageThumbs.jsm:116:22
BackgroundPageThumbs._onCaptureOrTimeout@resource://gre/modules/BackgroundPageThumbs.jsm:305:7
Capture.prototype._done/done@resource://gre/modules/BackgroundPageThumbs.jsm:451:7
Capture.prototype._done@resource://gre/modules/BackgroundPageThumbs.jsm:471:7
Capture.prototype.notify@resource://gre/modules/BackgroundPageThumbs.jsm:428:5
 BackgroundPageThumbs.jsm:116
favicon not found for uri Promise-backend.js:940
Use of getPreventDefault() is deprecated.  Use defaultPrevented instead.  jquery.min.js:3:6598
[OneID]   [ERROR] :  Already initialized  DisneyID.js:6
a248.e.akamai.net : server does not support RFC 5746, see CVE-2009-3555  (unknown)
[OneID]   [ERROR] :  Session not established  DisneyID.js:6
Failed to execute ‘postMessage’ on ‘DOMWindow’: The target origin provided (‘http://www.espn.com’) does not match the recipient window’s origin (‘null’). inner
Using //@ to indicate sourceMappingURL pragmas is deprecated. Use //# instead  jquery-1.10.2.min.js:1
Dynamic page targeting: lpid=>  mi-wf-site-1.0.97-bba9956-min.js:1
A promise chain failed to handle a rejection. Did you forget to '.catch', or did you forget to 'return'?
See https://developer.mozilla.org/Mozilla/JavaScript_code_modules/Promise.jsm/Promise

Date: Wed Jan 18 2017 15:17:08 GMT-0800 (Pacific Standard Time)
Full Message: Error: page-thumbnail:error
Full Stack: observe@resource://gre/modules/BackgroundPageThumbs.jsm:116:22
BackgroundPageThumbs._onCaptureOrTimeout@resource://gre/modules/BackgroundPageThumbs.jsm:305:7
Capture.prototype._done/done@resource://gre/modules/BackgroundPageThumbs.jsm:451:7
Capture.prototype._done@resource://gre/modules/BackgroundPageThumbs.jsm:471:7
Capture.prototype.notify@resource://gre/modules/BackgroundPageThumbs.jsm:428:5
  BackgroundPageThumbs.jsm:116
mi_Ads.env.init adinfoVisible=null  mi-wf-site-1.0.97-bba9956-min.js:1
mistats.sendBeacon: Sent  finalizestats.js:1501
micb: Loading MAB  micb.js:17
mi_Ads.env.getOOM finished OOM=true <unavailable>  mi-wf-site-1.0.97-bba9956-min.js:1
buildSlots: actual OOM header= true OOM article =false ADINFO override =false pagelevel= sectfront  mi-wf-site-1.0.97-bba9956-min.js:1
adsOnPageLoad: 12  mi-wf-site-1.0.97-bba9956-min.js:1
adsAfterPageLoad: 0  mi-wf-site-1.0.97-bba9956-min.js:1
adsDeferred: 0  mi-wf-site-1.0.97-bba9956-min.js:1
Use of getPreventDefault() is deprecated.  Use defaultPrevented instead.  jquery-1.10.2.min.js:5:17117
An unbalanced tree was written using document.write() causing data from the network to be reparsed. For more information https://developer.mozilla.org/en/Optimizing_Your_Pages_for_Speculative_Parsing  container.html:16
An unbalanced tree was written using document.write() causing data from the network to be reparsed. For more information https://developer.mozilla.org/en/Optimizing_Your_Pages_for_Speculative_Parsing  container.html:16

Comment 10

[Justin Dolske [:Dolske]]

(In reply to :Gijs from comment #8)

> > > Login data scheme type not supported: 3 ChromeProfileMigrator.js:477
> 
> So, some clarification: this reported error is an error from the import,
> which works fine pre-patch. The issue is with undo-ing the import.

Right, just saying there should probably be a separate bug to see what login we're unable to import, and if we should actually handle it.


> I did also add error catching to the password undo bits because in theory
> there could be similar issues there, I guess.

Yeah. Anything in the imported data be changed/deleted between when it's imported and actually deleted, so the undo should be robust against deleting things that no longer exist.

> In general, I think I've had a
> change of heart about how this UI should work. [...]
> I think in practice that's a rubbish user experience

Concur. ;-)


> > Thirdly, I think I saw comments elsewhere that from debugging to saw this
> > throwing when undoing History in Justin's VM? Do we know what was actually
> > happening and why?
> 
> So, we didn't see it actually throwing, but we saw it starting to import and
> not finishing.

Did it actually stall/stop, or was it still progressing?

I guess we don't wait for the autoimport to finish before showing the Undo bar (or else it never would have been shown?)... Do we correctly handle the user invoking Undo before the import has completed. [Which doesn't seem to be happening here, since I'd expect that symptom to be the initial history being removed, and then replaced with history that just got imported?]

Comment 11

[Justin Dolske [:Dolske]]

Comment on attachment 8828032 [details]
Bug 1331800 - catch errors from history removals and don't block undo on them,

https://reviewboard.mozilla.org/r/105560/#review106510

Comment 12

[Justin Dolske [:Dolske]]

(In reply to :Gijs from comment #1)

> > https://jwilliams-softvision.tinytake.com/sf/MTI3MzY4M180Nzc2OTEy
> > 
> > As you can see it takes about 1 minute for my tabs, browser history, etc. to
> > be automigrated over from Chrome, which freezes my browser.
> 
> Yes, this is a known issue. See bug 1289217. I expect there is quite a lot
> of history in the chrome profile...

Hmm, AIUI bug 1289217 and bug 1249008 were more about there being a bad hang when the Places Library window was open (I guess due to it observing and handling batch changes inefficiently). I was under the impression that while the import could still take a while to complete, we'd still expect the browser to be fairly responsive while it's running. Is that not the case?

From a quick code skim (but no profiling!) it does seem like there's some work we could do to improve things. In particular I wonder if huge pile of Promises we dispatch (wrapping MigrationUtils.insertVisitsWrapper) are swamping the event loop. New bug?

Comment 13

[:Gijs (he/him)]

(In reply to Justin Dolske [:Dolske] from comment #10)
> (In reply to :Gijs from comment #8)
> 
> > > > Login data scheme type not supported: 3 ChromeProfileMigrator.js:477
> > 
> > So, some clarification: this reported error is an error from the import,
> > which works fine pre-patch. The issue is with undo-ing the import.
> 
> Right, just saying there should probably be a separate bug to see what login
> we're unable to import, and if we should actually handle it.

Yes. Filed bug 1332219

> > In general, I think I've had a
> > change of heart about how this UI should work. [...]
> > I think in practice that's a rubbish user experience
> 
> Concur. ;-)
> 
> 
> > > Thirdly, I think I saw comments elsewhere that from debugging to saw this
> > > throwing when undoing History in Justin's VM? Do we know what was actually
> > > happening and why?
> > 
> > So, we didn't see it actually throwing, but we saw it starting to import and
> > not finishing.
> 
> Did it actually stall/stop, or was it still progressing?

Ugh, sorry, I misspoke in my haste - I meant *starting to undo*.

As for progress, it was adjusting visit counts for 7000-odd entries. We stepped through the loop a few times and it seemed to be fine then, but the loop never exited. I didn't step through to find out exactly where in the 7000 items (I mean, that's a lot of stepping, over vidyo on a vanilla build...) we were getting stuck, but AIUI from Justin just waiting 10 minutes or whatever didn't finish the undo.

> I guess we don't wait for the autoimport to finish before showing the Undo
> bar (or else it never would have been shown?)...

We do, sorry for the confusion. We won't show the undo bar before the import finishes.

bug 1331888 handles not re-showing the undo notification even if the undo gets 'stuck'.

(In reply to Justin Dolske [:Dolske] from comment #12)
> (In reply to :Gijs from comment #1)
> 
> > > https://jwilliams-softvision.tinytake.com/sf/MTI3MzY4M180Nzc2OTEy
> > > 
> > > As you can see it takes about 1 minute for my tabs, browser history, etc. to
> > > be automigrated over from Chrome, which freezes my browser.
> > 
> > Yes, this is a known issue. See bug 1289217. I expect there is quite a lot
> > of history in the chrome profile...
> 
> Hmm, AIUI bug 1289217 and bug 1249008 were more about there being a bad hang
> when the Places Library window was open (I guess due to it observing and
> handling batch changes inefficiently). I was under the impression that while
> the import could still take a while to complete, we'd still expect the
> browser to be fairly responsive while it's running. Is that not the case?

Hm, yes, that's a fair point. I think everything currently runs on the main thread though. So there's basically contention for the main thread's event loop, I guess, so responses to e.g. clicking buttons are super slow. We could try to make that import let the MT 'breathe' more, though that will of course make the overall process take longer. Not sure what the best course of action is here; doing what Marco suggested and limiting our imports might also be helpful. I haven't focused on the import so much, I'm afraid. But yes...

> New bug?

:-)

Filed bug 1332225.

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7400d6afc2d2

Comment 15

[:Gijs (he/him)]

Comment on attachment 8828032 [details]
Bug 1331800 - catch errors from history removals and don't block undo on them,

Approval Request Comment
[Feature/Bug causing the regression]: funnelcake 99/100/...
[User impact if declined]: undoing the automated migration can get stuck, leading to a very confusing user experience
[Is this code covered by automated tests?]: yes, and this patch adds more tests.
[Has the fix been verified in Nightly?]: it's been verified by QE with a custom local build with the patch on top of nightly, yes.
[Needs manual test from QE? If yes, steps to reproduce]: see comment #0 - QE reported this. :-)
[List of other uplifts needed for the feature/fix]: n/a
[Is the change risky?]: no
[Why is the change risky/not risky?]: this once more only affects the funnnelcake. Risk for the funnelcake is also very very low, because this is just adding some try...catches, plus tests.
[String changes made/needed]: no.

I realize we might not spin an rc3 so I don't know if this can make it, but I figured asking for approval was better than not asking. Either way it'd be worth having this in aurora if we eventually end up shipping this stuff.

Comment 16

[Julien Cristau [:jcristau]]

Comment on attachment 8828032 [details]
Bug 1331800 - catch errors from history removals and don't block undo on them,

fix for automigration undo, let's take this in aurora52 at least

Comment 17

[Liz Henry (:lizzard) (relman/hg->git project)]

This missed the RC2 build and I'm not planning RC3 at this point. I'm not sure if there's a way we can work around that for the funnelcake build.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/2cffdc392dc8

Comment 19

[:Gijs (he/him)]

Tweaking summary to be a bit clearer.

Comment 20

[:Gijs (he/him)]

Hey Justin, can you recheck if you're still seeing this issue as-is with the 51 rc builds on the win8 machine where you reproduced this before, given they don't contain the fix here? If so, can we find a time to look at this again together one of the next few days? Feels like we're still missing something about what's broken here.

Comment 21

[Justin [:JW_SoftvisionQA]]

Yes Gijs I still do see this issue on the 51 rc build. We can look at this sometime tomorrow if you would like.

Comment 22

[:Gijs (he/him)]

(In reply to Justin [:JW_SoftvisionQA] from comment #21)
> Yes Gijs I still do see this issue on the 51 rc build. We can look at this
> sometime tomorrow if you would like.

Perfect, sending you a calendar invite. Thanks!

Comment 23

[Julien Cristau [:jcristau]]

Comment on attachment 8828032 [details]
Bug 1331800 - catch errors from history removals and don't block undo on them,

Clearing beta approval flag, beta is now 52 which got this patch during aurora.

Comment 24

[Gerry Chang [:gchang]]

Comment on attachment 8828032 [details]
Bug 1331800 - catch errors from history removals and don't block undo on them,

We don't have plan to have 51 dot release now. Rel51-.