2019936 - Remove the IE importer and tests

Status: RESOLVED FIXED

(Firefox :: Migration, task, P3)

Description

[Mike Conley (:mconley) (:⚙️)]

The Internet Explorer migrator was disabled by default starting in Firefox 146, and we haven't heard about or come up with any reasons for keeping it around. I think we can safely remove it now.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

I believe this is straight-forward removal and can probably be done as a good-first-bug by a motivated contributor.

It's best if the contributor is on Windows to ensure that the changes to our Windows migration native code continues to compile locally.

Steps for removal:

1. Remove the pref here
2. Remove the IEProfileMigrator registration here
3. Remove it from the build package here
4. Then this migrator class can be removed entirely
5. As well as its documentation

Then we can remove the nsIEHistoryEnumerator:

1. Remove this file and its build registration and linting exception.
2. Also remove its header file
3. Stop registering it as an XPCOM component here

Then we need to update tests:

1. This test should be renamed to "browser_edge_bookmarks_success_strings.js", the IE enablement should be removed and the "ie" migrator removed from the list to test.
2. These two tests can be removed, along with their registration.
3. This whole directory can be removed: browser/components/migration/tests/unit/insertIEHistory, as well as this linting exception and build registration

That should get us a good chunk of the way. We may discover that there's more to remove after this is done, but I suspect this is the bulk of it.

Comment 2

[Oluwatobi]

Is this bug available for Outreachy applicants to work on? If so, please assign it to me.

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Hi, Oluwatobi! Yes, this is available to Outreachy applicants. Go ahead and submit a patch, and once you've got your first revision up, I'll mark you as the assignee. :)

Comment 4

[Oluwatobi]

Attachment: Bug 2019936 - Remove preference configuration for the IE importer feature. r=nsharpley,kcochrane,mconley

This is part of the cleanup steps to fully remove the internet explorer migrator feature from FireFox

Comment 5

[Oluwatobi]

Attachment: Bug 2019936 - Remove the redundant IEProfileMigrator.sys.mjs file as part of the Internet Explorer migrator feature cleanup. r=nsharpley,kcochrane,mconley

Comment 6

[Oluwatobi]

Attachment: Bug 2019936 - Remove nsIEHistoryEnumerator and its related entries as part of the Internet Explorer migrator feature cleanup steps. r=nsharpley,kcochrane,mconley,Gijs

Comment 7

[Oluwatobi]

Attachment: Bug 2019936 - Remove test related to the Internet Explorer migrator as part of the Internet Explorer migrator feature cleanup steps. r=nsharpley,kcochrane,mconley

Comment 8

[Oluwatobi]

Thank you, @Gijs, for reviewing Patch D289582. Could you advise if any action is required on my part to address reviewbot’s android-fenix (Mozlint) defects warning?

    2 defects found by android-fenix (Mozlint)

IMPORTANT: Found 2 defects (error level) that must be fixed before landing.

You can run this analysis locally with:

    ./mach lint --warnings --outgoing

The analysis task source-test-clang-tidy failed, but we could not detect any defect.
Please check this task manually.

The analysis task source-test-mozlint-clippy failed, but we could not detect any defect.
Please check this task manually.

The analysis task source-test-clang-external failed, but we could not detect any defect.
Please check this task manually.

I ran ./mach lint --warnings --outgoing both before and after pushing upstream, but the ESLint warning still appears locally:

 0   warning  File ignored because of a matching ignore pattern. Use "--no-ignore" to disable file ignore settings or use "--no-warn-ignored" to suppress this warning.  (eslint)

Running ./mach lint -l android-fenix --outgoing also throw this error:

ERROR: all linters were skipped due to setup, nothing to do!
Sentry is attempting to send 1 pending events
Waiting up to 2 seconds
Press Ctrl-Break to quit

I used ./mach bootstrap to configure GeckoView/Firefox for Android Artifact Mode, but the android-fenix linter still did not work.

Is there anything further I should do to prevent this warning in future patch submissions?

Comment 9

[Oluwatobi]

Attachment: Bug 2019936 - Stop packaging the IE browser image. r=nsharpley,kcochrane,mconley

This is part of the Internet Explorer migrator feature cleanup.

Comment 10

[Oluwatobi]

Attachment: Bug 2019936 - Fix the Android linting errors in the android-fenix (Mozlint) codebase r=nsharpley,kcochrane,mconley,Gijs

I removed the contentSizeInBytes argument from getString to
ensure that it retrieves plain text. The Android linter identified
mozac_feature_downloads_again_dialog_title as an invalid format
string; therefore, it should not be used with String.format
(StringFormatInvalid).

I also removed the create_tab_group_name_label string because the
linter reported that R.string.create_tab_group_name_label is
unused (UnusedResources).

Ref:

• https://treeherder.mozilla.org/logviewer?job_id=555580127&repo=try&task=VyyeqpXURgqtFNQEf4ecWg.0&lineNumber=7414

• https://phabricator.services.mozilla.com/D289582

• https://phabricator.services.mozilla.com/D289584

• https://phabricator.services.mozilla.com/D289909

Comment 11

[:Gijs (he/him)]

(In reply to Oluwatobi from comment #8)

Thank you, @Gijs, for reviewing Patch D289582. Could you advise if any action is required on my part to address reviewbot’s android-fenix (Mozlint) defects warning?

Hi, sorry, I'm just seeing this now. No, you don't need to do anything.

Is there anything further I should do to prevent this warning in future patch submissions?

You can try rebasing your patches to current firefox-main. But in general I wouldn't worry about them.

Comment 12

[Oluwatobi]

(In reply to :Gijs (he/him) from comment #11)

You can try rebasing your patches to current firefox-main. But in general I wouldn't worry about them.

Thanks for clarifying. I’ll go ahead and rebase the patches as you suggested.

Also, I get a warning message whenever I run the ./mach lint -o --fix command:

$ ./mach lint -o --fix --warnings
C:/mozilla-source/firefox/browser/app/profile/firefox.js
  0   warning  File ignored because of a matching ignore pattern. Use "--no-ignore" to disable file ignore settings or use "--no-warn-ignored" to suppress this warning.  (eslint)

✖ 1 problem (0 errors, 1 warning, 0 fixed)

I tried to format the firefox.js file with Prettier, but it’s excluded by the .prettierignore file:

# browser/ exclusions
browser/app/
browser/branding/**/firefox-branding.js

As a result, the formatter does not apply to this file. Could you advise on how to proceed with linting in this case? Patches D289582, D289584, and D289909 are currently blocked due to this issue.

Comment 13

[:Gijs (he/him)]

You can ignore the warning, we deliberately do not format firefox.js because the parser for that file is not actually a normal JS parser so arbitrary formatting changes could break it.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/309b8253559b
https://hg.mozilla.org/integration/autoland/rev/aa140252e62f
Remove preference configuration for the IE importer feature. r=mconley
https://github.com/mozilla-firefox/firefox/commit/b1e9e4f9bb90
https://hg.mozilla.org/integration/autoland/rev/62096d30648c
Remove the redundant IEProfileMigrator.sys.mjs file as part of the Internet Explorer migrator feature cleanup. r=mconley,migration-reviewers
https://github.com/mozilla-firefox/firefox/commit/581e3556ff15
https://hg.mozilla.org/integration/autoland/rev/47e18569fe42
Remove nsIEHistoryEnumerator and its related entries as part of the Internet Explorer migrator feature cleanup steps. r=nsharpley,mconley,migration-reviewers,Gijs
https://github.com/mozilla-firefox/firefox/commit/dc3ebab46ae8
https://hg.mozilla.org/integration/autoland/rev/b218c503ea66
Remove test related to the Internet Explorer migrator as part of the Internet Explorer migrator feature cleanup steps. r=mconley,migration-reviewers
https://github.com/mozilla-firefox/firefox/commit/8a02b884deed
https://hg.mozilla.org/integration/autoland/rev/21eaeebcc046
Stop packaging the IE browser image. r=nsharpley,mconley,migration-reviewers

Comment 15

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/aa140252e62f
https://hg.mozilla.org/mozilla-central/rev/62096d30648c
https://hg.mozilla.org/mozilla-central/rev/47e18569fe42
https://hg.mozilla.org/mozilla-central/rev/b218c503ea66
https://hg.mozilla.org/mozilla-central/rev/21eaeebcc046