995095 - Move the nsPlacesAutocomplete component to Suite (comm-central part)

Status: RESOLVED FIXED

(SeaMonkey :: Autocomplete, defect)

Description

[Marco Bonardo [:mak]]

Once we are satisfied with unified complete, and it's unlikely to be disabled, we should remove the 2 old components from the tree.

Comment 1

[Octoploid]

Please don't, or you'll probably lose the rest of your market share.

Comment 2

[Marco Bonardo [:mak]]

let's convert this bug to remove the unifiedcomplete pref and stop bundling the old components in Places.

Comment 3

[Marco Bonardo [:mak]]

(In reply to Marco Bonardo [::mak] from comment #2)
> in Places.

ehr, I meant "in Firefox".
Then it would be nice if Seamonkey could move them to the suite repo, since it's unlikely we can maintain them working, not using them anymore. But this requires a separate bug.

Comment 5

[arni2033]

I'm sorry, are you going to delete that pref anyway, no matter how many "edgecases" are left aside?
The same happened with the searchbar, and now we got unusable thing. Maybe it's not necessary to completely delete/KILL/exterminate! what worked tolerably for years? I believe the pref should stay
http://ssmaker.ru/661fb42e.png

Comment 6

[Marco Bonardo [:mak]]

(In reply to arni2033 from comment #5)
> I'm sorry, are you going to delete that pref anyway, no matter how many
> "edgecases" are left aside?

We have a list of issues by priority. Currently there's only a single P1 that makes the experience a little bit annoying, but we didn't get a lot of reports of broken cases, the ones we got are things that would have been broken in the old component as well. If there's a very broken case you think makes the experience horrible, feel free to express that and we'll evaluate that.

> The same happened with the searchbar, and now we got unusable thing. Maybe
> it's not necessary to completely delete/KILL/exterminate!

I think it's two very different cases that have been handled in different ways. UnifiedComplete has been enabled in Nightly from version 39 to 43, and will ride the whole Aurora and Beta train to Release in 43. Thus it got a huge amount of testing everyday (it's one of the more prominent ui pieces) in the last many months. It's also likely this removal will happen at least 2 releases from now. That's a lot of time to collect feedback and improve things yet, so I think there's not much to worry about.

The only thing is common is the reason to remove the old components at a certain point; the burden to maintain 2 completely parallel code bases for the same feature is excessive for the current available resources, one of the two (the non default one) would shortly begin to collect bugs and could stop working at any time, because it's not tested by automation and nobody has the time for manual testing.

Comment 7

[Christian Kujau]

Is this bug to remove the browser.urlbar.unifiedcomplete preference? Please don't. Firefox v43 introduced a rather irritating "Visit Site..." dropdown in the URL bar and this preference was the only knob to turn this off.

Comment 8

[Marco Bonardo [:mak]]

(In reply to Christian Kujau from comment #7)
> Is this bug to remove the browser.urlbar.unifiedcomplete preference? Please
> don't. Firefox v43 introduced a rather irritating "Visit Site..." dropdown
> in the URL bar and this preference was the only knob to turn this off.

You can just use a userChrome.css customization to hide it if it's so annoying to you. But I'd suggest to try to get used to it first.

Comment 9

[Julien L.]

(In reply to Marco Bonardo [::mak] from comment #8)
> (In reply to Christian Kujau from comment #7)
> > Is this bug to remove the browser.urlbar.unifiedcomplete preference? Please
> > don't. Firefox v43 introduced a rather irritating "Visit Site..." dropdown
> > in the URL bar and this preference was the only knob to turn this off.
> 
> You can just use a userChrome.css customization to hide it if it's so
> annoying to you. But I'd suggest to try to get used to it first.

It is right that this "Visit site" dropdown or "search with ..." stuff is completely useless.
I don't hide it for now but it's an useless function.

Comment 10

[Bob49]

and say that there at some time, had been successful, in order to disable the search engine via a url in the bar! And it comes just arrived for Release ! This is a regression for freedom of the user !!!!

@ Marco Bonardo, and when using userChrome.css will be removed? .. ... Freedom and less ...

the choice of research opportunities in the url bar are very good right now.

Comment 11

[Christian Kujau]

(In reply to Marco Bonardo [::mak] from comment #8)
> You can just use a userChrome.css customization to hide it if it's so
> annoying to you. But I'd suggest to try to get used to it first.

Can you please share the magic .css code how to disable the new URL layout? Thanks!

Comment 12

[Marco Bonardo [:mak]]

Morphing the bug since the Firefox part will happen in bug 1223728.

nsPlacesAutocomplete won't be anymore part of Places, for now I'm gating it behind a if CONFIG['MOZ_SUITE'] but it should really be moved to the suite itself along with the tests under unit/tests/autocomplete.

Comment 13

[Marco Bonardo [:mak]]

note that alternatively you can move to UnifiedComplete.js, you should be able to obtain similar results as the old search setting some prefs (like disabling autoFill, not setting "enable-actions" searchparam...)

Comment 14

[Philip Chee]

(In reply to Marco Bonardo [::mak] from comment #13)
> note that alternatively you can move to UnifiedComplete.js, you should be
> able to obtain similar results as the old search setting some prefs (like
> disabling autoFill, not setting "enable-actions" searchparam...)

That would be optimum however I don't know how to switch to that. Is there a Dummies Guide?

Comment 15

[Marco Bonardo [:mak]]

replace "history" and set browser.urlbar.autoFill to false (but looks like it's already the default), may be enough. But you should test it.
Looks like you are exposing autoFill http://mxr.mozilla.org/comm-central/source/suite/common/pref/pref-locationbar.xul#34 so maybe you want to try both cases.
The fact the inputfield doesn't have "enable-actions" in the searchparams attribute should filter out most of the special unifiedcomplete actions, so you should be fine (there may be bugs I don't know, for example I just fixed search suggestions to be skipped in that case in Nightly).

So I think it's mostly matter of testing.

Comment 16

[Philip Chee]

Attachment: patch v1 comm-central

See Bug 1281005 attachment 8763697 [details] [diff] [review] for the mozilla-central part. Unless you want to import the history too...

Comment 17

[Philip Chee]

Attachment: filemap.txt import/convert nsPlacesAutocomplete with history

Comment 18

[Philip Chee]

Attachment: Patch v2 changes to moz.build and test files after history import

moz.build and unit test changes.

Comment 19

[Philip Chee]

Attachment: mercurial bundle containing the intermediate repository created by Mercurial Convert

Comment 20

[Philip Chee]

Attachment: nsPlacesAutocomplete.zip

Contents of nsPlacesAutocomplete.zip:

 1/09/2016   0:49       3,966,463  places-inbound.bundle
 1/09/2016   0:52         158,934  suite-inbound.bundle
26/08/2016  23:33              99  filemapA.txt
27/08/2016  23:44             847  filemapB.txt
31/08/2016  23:41              84  splicemap.txt
 1/09/2016   0:29             166  splicemapB.txt
 1/09/2016   0:57              66  splicemapC.txt
 1/09/2016   1:04           1,733  notes1.txt

notes:

Step 1:
hg convert --filemap filemapA.txt --config convert.skiptags=true mozilla-inbound places-inbound
Step 2:
hg convert --filemap filemapB.txt --splicemap splicemapB.txt --config convert.skiptags=true places-inbound  suite-inbound2

Step 3: rebase/linearize on suite-inbound2:
hg strip -r 197
hg strip -r 141
rebase .... etc

Step 4: land on comm-central:
hg convert --splicemap splicemapC.txt --config convert.skiptags=true  suite-inbound2  comm-aurora

filemapA.txt:
include toolkit/components/places/
rename  toolkit/components/places/     suite/common/places/

filemapB.txt:
include    suite/common/places/src/nsPlacesAutoComplete.js
include    suite/common/places/src/nsPlacesAutoComplete.manifest
#
include    suite/common/places/nsPlacesAutoComplete.js
include    suite/common/places/nsPlacesAutoComplete.manifest
#
include    suite/common/places/tests/autocomplete/
include    suite/common/places/tests/autocomplete/
#
include    suite/common/places/tests/head_common.js
include    suite/common/places/tests/unit/test_416211.js
include    suite/common/places/tests/unit/test_416214.js
include    suite/common/places/tests/unit/test_417798.js
include    suite/common/places/tests/unit/test_418257.js
include    suite/common/places/tests/unit/test_422277.js
include    suite/common/places/tests/unit/test_multi_word_search.js
include    suite/common/places/tests/unit/test_word_boundary_search.js

splicemapB.txt:
e0144251aef999bf4a24f20ec8af020f9efd4257 30c920b20c526fe4fbff7cce43553fe7f495a286
35245876fac0bc646bf9bae6d002d75de3283115 0fdee75a305a09d1ff7f0799a4718db92368f3bc

splicemapC.txt:
first_changeset_of_suite-inbound2 last_changeset_of_comm-central

Comment 21

[Philip Chee]

> Step 4: land on comm-central:
> hg convert --splicemap splicemapC.txt --config convert.skiptags=true  suite-inbound2  comm-central

Hint: You might want to test this several times, using hg strip to get back to a clean comm-central tree. However this won't work in subsequent attempts unless you delete the comm-central/.hg/shamap file created by convert each time.

Comment 22

[Ian Neal]

Comment on attachment 8784329 [details] [diff] [review]
Patch v2 changes to moz.build and test files after history import

LGTM r/a=me

Comment 23

[Ian Neal]

Comment on attachment 8786842 [details]
nsPlacesAutocomplete.zip

Seems to have all the correct file in the right place f=me

Comment 24

[Ian Neal]

suite/common/places/moz.build will need a few tweaks:
LOCAL_INCLUDES
include
with Files

Comment 25

[Philip Chee]

Pushed nsPlacesAutocomplete with history import with history from mozilla-central:
Pushlog: http://hg.mozilla.org/comm-central/pushloghtml?fromchange=75c57c9dc363&tochange=9bd937fc0c12

Will push Part 2 (attachment 8784329 [details] [diff] [review]) once the removal patch hits mozilla-central
(https://hg.mozilla.org/integration/mozilla-inbound/rev/9792d02838ac)

Comment 26

[Philip Chee]

Patch to start building nsPlacesAutocomplete in Suite:
Pushed to comm-central
http://hg.mozilla.org/comm-central/rev/af0e89924350