Closed Bug 1487125 Opened 6 years ago Closed 6 years ago

Write basic Query Context and Address Bar Controller classes

Categories

(Firefox :: Address Bar, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 64
Tracking Status
firefox63 --- wontfix
firefox64 --- fixed

People

(Reporter: standard8, Assigned: standard8)

References

()

Details

(Whiteboard: [fxsearch])

User Story

- Create outlines of the new classes.
- Make the AddressBarController "work" to the extent that it can handle passing basic searches from the view to the model and return results (with stubbing of the model and view where necessary).

Attachments

(2 files)

As an early part of the address bar rewrite, we want to create some basic forms of QueryContext and AddressBarController, that we can add to as we develop the new address bar architecture. The main aims of this bug are outlined in the user story.
Depends on: 1487783
This is based around using promises for query completion. Depends on D4566
(In reply to Mark Banner (:standard8) from comment #2) > Created attachment 9006363 [details] > Bug 1487125 - Create a basic UrlbarController object for the address bar > rewrite. This is a WIP patch that is slightly different to the original design for the controller. It uses promises for the view side of the interface rather than a listener based system - Mike suggested this a couple of times, so I thought I'd play around with it. I think it might be OK, as long as we are happy that the controller will only ever pass the view a complete set of results, and there will be no option for results being passed in blocks. I believe that was a limitation we previously decided on, due to issues with updating the UI multiple times, but I can't find the docs for it. We could also use a promise on the model side of the controller as well, Marco what do you think about that? If you folks could take a look at the patch(es), and see what you think to the above limitation, and using promises, that would be very useful before I finalise this for review.
Flags: needinfo?(mak77)
Flags: needinfo?(dao+bmo)
Flags: needinfo?(adw)
I discussed with Marco on irc. He pointed out that we need to have the potential for multiple result sets to the UI per query, as this gives the minimum latency possible. Therefore the promise based approach isn't really sensible here. If we change how things work in the future, then we could revert to it, but I think it is simpler for now to have listener based approach.
Flags: needinfo?(mak77)
Flags: needinfo?(dao+bmo)
Flags: needinfo?(adw)
(In reply to Mark Banner (:standard8) from comment #3) > I think it might be OK, as long as we are happy that the controller will > only ever pass the view a complete set of results, and there will be no > option for results being passed in blocks. > > I believe that was a limitation we previously decided on, due to issues with > updating the UI multiple times, but I can't find the docs for it. That's not how I remember it. I thought incremental updates were a crucial requirement. I don't have all the context for why this would be crucial, though... I think basically some results can take longer to retrieve than others, and we want to give something to the user ASAP? Not doing incremental updates makes my life a bit easier, so if mak thinks this okay I'm not going to object.
Mark, I saw your comment in mid-air, submitted mine anyway. Seems like we're on the same page.
I think the patches are in a reasonable place now that we can start reviews. I think there's going to be lots of mutations as we get going, but hopefully this is a good base.
Status: NEW → ASSIGNED
Just to chime in, I agree with the other comments that multiple result sets and incremental updates are important.
Comment on attachment 9004928 [details] Bug 1487125 - Create a basic QueryContext object for the Address Bar architecture rewrite. Marco Bonardo [::mak] has approved the revision.
Attachment #9004928 - Flags: review+
Comment on attachment 9006363 [details] Bug 1487125 - Create a basic UrlbarController object for the address bar rewrite. Marco Bonardo [::mak] has approved the revision.
Attachment #9006363 - Flags: review+
Comment on attachment 9006363 [details] Bug 1487125 - Create a basic UrlbarController object for the address bar rewrite. Dão Gottwald [::dao] has approved the revision.
Attachment #9006363 - Flags: review+
Comment on attachment 9004928 [details] Bug 1487125 - Create a basic QueryContext object for the Address Bar architecture rewrite. Dão Gottwald [::dao] has approved the revision.
Attachment #9004928 - Flags: review+
Pushed by mbanner@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/fa6fb040f5b6 Create a basic QueryContext object for the Address Bar architecture rewrite. r=mak,dao
Pushed by mbanner@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/96254477baa8 Create a basic UrlbarController object for the address bar rewrite. r=mak,dao
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: