Make the history panel work

RESOLVED FIXED

Status

()

Firefox for iOS
General
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: wesj, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

46 bytes, text/x-github-pull-request
wesj
: review+
Details | Review | Splinter Review
(Reporter)

Description

3 years ago
Hook up the history data source to the panel
(Reporter)

Comment 1

3 years ago
Created attachment 8542660 [details] [review]
Pull request

Make the history panel work
Attachment #8542660 - Flags: review?(bnicholson)
Comment on attachment 8542660 [details] [review]
Pull request

Removing r? for now until these are rebased onto the non-Core Data implementation.
Attachment #8542660 - Flags: review?(bnicholson)
(Reporter)

Comment 3

3 years ago
Created attachment 8545720 [details] [review]
PR
Attachment #8542660 - Attachment is obsolete: true
Attachment #8545720 - Flags: review?(bnicholson)
(Reporter)

Comment 4

3 years ago
Created attachment 8546844 [details]
PR

Updated PR. I also reworked the one history test I have to be a little cleaner in here.
Attachment #8545720 - Attachment is obsolete: true
Attachment #8545720 - Flags: review?(bnicholson)
Attachment #8546844 - Flags: review?(bnicholson)
Comment on attachment 8546844 [details]
PR

Lots of comments in PR, but they're all pretty minor, so this mostly looks good to me.
Attachment #8546844 - Flags: review?(bnicholson) → review-
(Reporter)

Comment 6

3 years ago
Created attachment 8549930 [details] [review]
Pull request

I'm kinda confused about the r- and the "this looks ok" comment. Re-review if you want....
Attachment #8549930 - Flags: review+
(In reply to Wesley Johnston (:wesj) from comment #6)
> Created attachment 8549930 [details] [review]
> Pull request
> 
> I'm kinda confused about the r- and the "this looks ok" comment. Re-review
> if you want....

Heh, sorry -- I mostly r-'d because of the merge conflicts that were still in the commits, and just wanted to be sure those got fixed and rebased first. This looks fine (with the public accessors changed back to default, if possible).
Attachment #8546844 - Attachment is obsolete: true
(Reporter)

Comment 8

3 years ago
We've landed this now in https://bugzilla.mozilla.org/show_bug.cgi?id=1122224
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.