[Regression]'Add to Bookmarks' share extension does not work on v7.3b2756

VERIFIED FIXED

Status

()

Firefox for iOS
General
VERIFIED FIXED
9 months ago
9 months ago

People

(Reporter: csuciu, Assigned: st3fan)

Tracking

unspecified
Other
iOS

Firefox Tracking Flags

(fxios7.3, fxios-v7.2 unaffected, fxios-v7.3 verified)

Details

Attachments

(2 attachments)

(Reporter)

Description

9 months ago
v7.3b2756

1. Visit a site
2. Open Share menu and select => Firefox => Add to Bookmarks => Add
3. Check Bookmarks panel

Result: The site doesn't show up in the bookmarks list.

Note: This issue is not reproducible on v7.2 (release)
(Assignee)

Comment 1

9 months ago
Created attachment 8860376 [details]
Debug log from ShareTo
(Assignee)

Comment 2

9 months ago
The problem is as follows: insertBookmark(_:title:favicon:intoFolder:withTitle:) is an asynchronous function. It returns a Deferred. But that deferred gets lost in profile.shareItem() and is ignore. So the ShareTo app extension finishes and then closes the database connection too soon.
(Assignee)

Comment 3

9 months ago
Created attachment 8860403 [details] [review]
PR https://github.com/mozilla-mobile/firefox-ios/pull/2651

This patch makes `shareItem` return a `Success` (which is a `Deferred<Maybe<Void>>`) so that the caller can wait for asynchronous database operations to finish. Currently we just call `shareItem()` and then immediately `profile.shutdown()`, which is bad since we are still in the process of adding the bookmark. This patch makes it possible to properly wait.
Assignee: nobody → sarentz
Attachment #8860403 - Flags: review?(sleroux)
(Assignee)

Updated

9 months ago
Status: NEW → RESOLVED
Last Resolved: 9 months ago
Resolution: --- → FIXED
(Assignee)

Updated

9 months ago
tracking-fxios: ? → 7.3
(Assignee)

Comment 4

9 months ago
Landed on both v7.x and master
(Reporter)

Comment 5

9 months ago
Verifying as fixed in 7.3 (2772) release
Status: RESOLVED → VERIFIED
status-fxios-v7.3: affected → verified
Attachment #8860403 - Flags: review?(sleroux) → review+
You need to log in before you can comment on or make changes to this bug.