Closed Bug 1735140 Opened 3 years ago Closed 3 years ago

[macOS] Importing from Safari does not import bookmarks on startup (e.g. with --migration, for first-time users)

Categories

(Firefox :: Migration, defect, P2)

ARM64
macOS
defect

Tracking

()

VERIFIED FIXED
95 Branch
Tracking Status
firefox-esr78 --- wontfix
firefox-esr91 --- wontfix
firefox93 --- wontfix
firefox94 --- verified
firefox95 --- verified

People

(Reporter: csasca, Assigned: Gijs)

Details

Attachments

(3 files)

Affected versions

  • Firefox 94.0b4
  • Firefox 95.0a1

Affected platforms

  • macOS 11.6 (ARM)

Steps to reproduce

  1. Launch Firefox with -migration command or access library within Firefox and select "Import from another browser"

Expected result

  • Safari is listed as an option and can be imported from

Actual result

  • Safari is not an option to select

Regression range

  • Will see for one

Additional notes

  • The issue can be seen in the following attachment
  • Seems that Intel based systems (macbook 2015) with macOS 11.6.1 is not affected by the issue
  • Made sure that Firefox had Full Disk Access on Privacy tab in System Settings
Has Regression Range: --- → no
Has STR: --- → yes

I'm not sure what differences there will be for ARM macOS versus Intel. Can you provide a regression range to see if this ever worked?

Flags: needinfo?(catalin.sasca)

Tried with mozregression and went back to 52.0a1 (2016-10-09) and there was still only Chrome on the list of selections. Seems that this isn't a regression.

Flags: needinfo?(catalin.sasca)
Has Regression Range: no → ---
Priority: -- → P3

Romain, surfacing this for you to prio. AFAICT this means that Safari bookmarks import is completely broken on M1 macs. Do you want to prio any higher than P3?

Flags: needinfo?(rtestard)

I'm marking as a P2 to reflect the fact that this should be fixed in the next 2 nightly cycles.
Also marking as S2 since my understanding is that no work-arounds exist (feel free to correct me there if it does)

Severity: S3 → S2
Flags: needinfo?(rtestard)
Priority: P3 → P2

I just tried this on macOS Monterey Beta on an M1 Macbook Pro and could not reproduce the problem - importing from Safari was an option and worked fine. I tried it with a local build with ./mach run -migration.

(In reply to Markus Stange [:mstange] from comment #5)

I just tried this on macOS Monterey Beta on an M1 Macbook Pro and could not reproduce the problem - importing from Safari was an option and worked fine. I tried it with a local build with ./mach run -migration.

Is it possible you've already given the local build full disk access in some way? I don't have access to an M1 device but previous issues have been to do with whether we have access to the Safari folder and/or the bookmarks subfolder.

Flags: needinfo?(mstange.moz)

(I guess it's also possible that this situation has improved in Monterey...)

Has Safari ever been used on the machine where this is an issue? Part of me also wonders if there's an issue where there just isn't anything to import?

Flags: needinfo?(catalin.sasca)

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

Is it possible you've already given the local build full disk access in some way?

The import wizard asked me to pick the bookmarks.plist file (I think that was the name) in the file picker manually, so I don't think the build had full disk access. (Though I do realize that it means I didn't follow comment 0 completely.)

Flags: needinfo?(mstange.moz)

(In reply to Markus Stange [:mstange] from comment #9)

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

Is it possible you've already given the local build full disk access in some way?

The import wizard asked me to pick the bookmarks.plist file (I think that was the name) in the file picker manually, so I don't think the build had full disk access. (Though I do realize that it means I didn't follow comment 0 completely.)

That's reassuring though, that's the experience (clunky as it is) that I would have expected on macOS 10.15 and later (x-ref bug 1493103). Thanks for checking!

I'm guessing there's something specific happening on QA's machine(s) but I'm not sure what it is...

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

Has Safari ever been used on the machine where this is an issue? Part of me also wonders if there's an issue where there just isn't anything to import?

Hello! I tested on the same machine and same macOS profile and Safari appears in the Import Wizard after adding some bookmarks inside Safari (I think this may be the same as bug 1723131). The only thing is that it does not import anything if it's a new macOS profile with -migration command (attached error). Importing works if it is done inside the browser by using the Import Data from another browser function and the -migration command work as well afterward. So the steps will be:

  1. Create a new macOS profile and have some bookmarks added inside Safari.
  2. Open Firefox with -migration command and choose Safary =no bookmarks are imported.
  3. Inside Firefox use Import data from another browser from Safari (works).
  4. Open again Firefox with -migration on another profile and bookmarks are imported now. Screen recording. (sorry for the long video, the default profile was selected xD).
Flags: needinfo?(catalin.sasca)

OK, I think I might know what's going on here. In the video, we prompt for permission to the Safari bookmarks file when importing from within Firefox, but not on the startup import. We need to also ask for permission when importing on startup. I found a bug that I think is likely the cause of this, and I suspect this also affects non-ARM macs.

Alexandru, I have a build with a bunch of debug logging added and an attempt at a fix. Can you try this build on a new macOS profile and check if it fixes the startup import case? https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/LVmY5ZtYTfG9q3DvhTO_Hw/runs/0/artifacts/public/build/target.dmg (if not, please check the browser console output and provide a screenshot / copy/paste, including stack traces for the messages)

Flags: needinfo?(alexandru.trif)
Summary: [macOS 11] Importing from Safari is not an option on mac ARM systems → [macOS 11] Importing from Safari does not work on startup (e.g. with --migration)
Attached file macOScrash.txt

Hello Gijs! Unfortunately, I cannot open the Nightly app provided on macOS 11 ARM (tried on two devices), and I can only open it on Intel macOS 11 and 10.15. On Intel platforms, the permission window is presented on the new macOS profile, and selecting bookmarks.plist from Safari imports the bookmarks.

Also, I attached the macOS error report from the ARM device, I hope it helps.

Flags: needinfo?(alexandru.trif)

(In reply to Alexandru Trif, QA [:atrif] from comment #13)

Created attachment 9247013 [details]
macOScrash.txt

Hello Gijs! Unfortunately, I cannot open the Nightly app provided on macOS 11 ARM (tried on two devices),

Ugh, sorry. Is it gatekeeper or something else? Maybe the artifact build doesn't have arm binaries? I ran a full arm try build, hopefully that will work? https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/FUbROr5MQGWsFBeEsDq3Xg/runs/0/artifacts/public/build/target.dmg

Flags: needinfo?(alexandru.trif)

Sorry Gijs, it seems that the build from comment 12 was the good one. After some digging and digging I found out that macOS ARM can open these builds only if Open with Rosetta is enabled inside Get Info. Given the fact that this needs a Terminal as well, the terminal must be opened as well in Rosetta. If Rosetta is not used there will be the error uploaded in comment 13 and then a message stating that You do not have permission to open application Firefox Nightly when trying to open it. Sorry again for not knowing this the first time. I could not open build from comment 14 at all either on macOS 11 ARM or on Intel 10.15 (it requires macOS 11 to run).

Anyway, the build from comment 12 displays the permission window on macOS 11 ARM as well, and selecting bookmarks.plist from Safari imports the bookmarks as expected. If there is anything I can help with please let me know. Thanks!

Flags: needinfo?(alexandru.trif)

OK, thanks! I'll put up an actual patch, then. :-)

Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED

Startup migrations pass the "ALL" constant, which unlike what you might expect is
not just an OR'd together set of all the other flags - so it needs to be checked
explicitly.

Comment on attachment 9247296 [details]
Bug 1735140 - also prompt for bookmark access permission for Safari on startup migrations, r?mak

Beta/Release Uplift Approval Request

  • User impact if declined: Migration doesn't get your Safari bookmarks
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: See previous comments, esp. comment #11
  • List of other uplifts needed: n/a
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): 1-line addition to the condition under which we check for permission to Safari data

(Note: this hasn't landed on Nightly yet but it has been successfully verified already using try builds, see previous comments)

  • String changes made/needed: None
Attachment #9247296 - Flags: approval-mozilla-beta?
Flags: qe-verify+
Summary: [macOS 11] Importing from Safari does not work on startup (e.g. with --migration) → [macOS] Importing from Safari does not import bookmarks on startup (e.g. with --migration, for first-time users)
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/fa47dd82af1b
also prompt for bookmark access permission for Safari on startup migrations, r=mak

Comment on attachment 9247296 [details]
Bug 1735140 - also prompt for bookmark access permission for Safari on startup migrations, r?mak

Gijs assures me that this is exceedingly low risk and unlikely to cause issues. Given that we're marketing Fx94 to macOS users for its battery life improvements, having a better Safari migration story in place makes sense. Approved for 94.0rc1.

Attachment #9247296 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 95 Branch
QA Whiteboard: [qa-triaged]

Verified fixed with Firefox 95.0a1 (20211025214106) and 94.0 (20211025220926) on macOS 11.6 ARM and macOS 10.15. The permission window is displayed on a new macOS profile if Firefox is opened in -migration command.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: