Closed
Bug 1071344
Opened 10 years ago
Closed 10 years ago
Breakdown: create a plan for autocomplete improvements
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
RESOLVED
FIXED
Iteration:
36.2
People
(Reporter: Dolske, Assigned: Unfocused)
References
Details
There's a lot of autocomplete plumbing improvement work under discussion, because we're going to need it to implement some of the actual features we'd like to ship in the product. This area has been challenging in the past, and it's a substantial amount of work, so we want to be deliberate about doing it.
So, the next step is to create a somewhat more formal document, drawing from existing discussions to create a plan of record that can be reviewed and approved by all involved. Basically it should include what our goals are (why we're doing this work, e.g. existing features in the backlog and to-be-filed UX work), and a detailed breakdown of the work that needs to be done to support that work (e.g. refactorings, improvements, bugfixes, etc etc).
Flags: firefox-backlog+
Updated•10 years ago
|
Flags: qe-verify-
Comment 1•10 years ago
|
||
I just filed bug 1071461 to track current work (it's still wip, I will add more dependencies as soon as I find them)
Depends on: 1071461
Updated•10 years ago
|
Points: --- → 5
Updated•10 years ago
|
Points: 5 → 8
Updated•10 years ago
|
Status: NEW → ASSIGNED
Updated•10 years ago
|
Iteration: 35.3 → 36.1
Assignee | ||
Comment 2•10 years ago
|
||
I have a cunning plan!
https://etherpad.mozilla.org/AwesomePlan
Assignee | ||
Comment 3•10 years ago
|
||
Marco/Paulo: Comments/thoughts/modifications/additions/removals?
Flags: needinfo?(paolo.mozmail)
Flags: needinfo?(mak77)
Comment 4•10 years ago
|
||
Would be great to set up a meeting to discuss this with higher bandwidth!
I like the "features wanted" section with the indications about which features are blocked.
About the refactoring, rewriting the controller in JavaScript is mentioned, but this looks like something that can't proceed in parallel with other features. This can work, but what is the feasibility of forking the current C++ code and interfaces in "browser" as a start? I remember some discussion but I don't remember a definitive answer.
Flags: needinfo?(paolo.mozmail)
Assignee | ||
Comment 5•10 years ago
|
||
(In reply to :Paolo Amadini from comment #4)
> This can work, but what is the feasibility of forking the current
> C++ code and interfaces in "browser" as a start? I remember some discussion
> but I don't remember a definitive answer.
Ah, I should have added a section on that. I looked into forking the existing c++ implementation in bug 1067935, on the assumption that it would allow us some quick fixes. Unfortunately, it ends up being not so simple (and therefore not a quick and easy win) due to string APIs, duplicating IDL interfaces, and assumptions in other code. So it's doable, but I think it changes the trade-off.
One of the goals I've been aiming at is making code more maintainable and much quicker/easier to implement/change things. IMO, a new JS implementation gets us further along that continuum, and will be easier to do. I don't think a basic JS implementation will take very long, and then it will be much quicker to iterate on.
I had been considering having a fork of the existing implementation a temporary solution for us to make some short-term fixes. But I no longer think that's worth it - given the the difficulty in both forking and of then actually fixing specific issues (such as key handling).
Assignee | ||
Comment 6•10 years ago
|
||
(In reply to Blair McBride [:Unfocused] from comment #5)
> Ah, I should have added a section on that.
And so I did.
Updated•10 years ago
|
Iteration: 36.1 → 36.2
Comment 7•10 years ago
|
||
I'm not directly editing the etherpad cause I really don't want to break the awesome work you did, feel free to merge these if you think they have value.
Assuming we don't want to add further features, I think the current work section (MILESTONE 1) is missing some pieces:
SEARCH IMPROVEMENTS, already enabled but not working nicely:
1. Search provider top suggestion is nonsense (bug 958188). We should at a minimum make it opt-in so we do it only for certain engines, as a I suggested in https://bugzilla.mozilla.org/show_bug.cgi?id=958188#c13
2. Search results in the popup are parsed back into engine + search terms, unfortunately that's not working very well at the moment, we need to deduplicate results (bug 1063542, bug 1075662). Plus we likely need a new bug cause I noticed if you search "word" in google, and visit 10 pages of results, you get 10 *identical* entries stating "google word".
POLISH:
a. default empty favicon sucks in autocomplete (bug 1074937)
b. jumping text on external non-retina display (Bug 1088000) (could be a layout bug)
I think the above are bad enough that they could be part of Milestone 1, rather than Milestone 2.
NOT BLOCKERS, that could be moved to a future milestone:
I. bug 1040340 is not really "needed", it's a nice to have code cleanup, but we can move on without it and it's a mentored bug.
II. bug 1054816 is a very nice to have, but could become part of the refactoring part rather than being a requirement for milestone 1. I don't see a reason we need this immediately when we plan a refactor.
MILESTONE 2.
I think most of the features here could be postponed to milestone 4, with the purpose of first having a nicer codebase to work with.
Speaking with UX they said what they really care about (more than other things) is search suggestions, that in your plan ended up in milestone 4 instead.
I think milestone 2 should be:
search suggestions, search query in urlbar, fix purposes, remaining results de-duping.
Anything else CAN wait afaict.
All the rest looks good.
I just didn't find (but could be I missed it) the very compelling request that controller stops trying to be smart with search results. Controller should do the controller: get results from a search provider and pass them to a view, and handle events clearly. Stop doing comparisons and looking into matches values. That's the view's job.
well done!
Flags: needinfo?(mak77)
Assignee | ||
Comment 8•10 years ago
|
||
Calling this fixed, after discussions and refinements during/after meeting.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•