Closed Bug 534663 Opened 15 years ago Closed 5 years ago

[ubuntu] updates overwrite/erases defined keywords for default search engines

Categories

(Firefox :: Search, defect)

3.5 Branch
x86_64
Linux
defect
Not set
minor

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: samiaira, Unassigned)

References

Details

(Whiteboard: [3.6.x])

Attachments

(1 file)

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.7pre) Gecko/20091212 Ubuntu/8.10 (intrepid) Shiretoko/3.5.7pre
Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.7pre) Gecko/20091212 Ubuntu/8.10 (intrepid) Shiretoko/3.5.7pre

Every time update is applied, all the keywords for default search engines are erased. Not critical or even important, but highly annoying.

Reproducible: Always

Steps to Reproduce:
1. apply update
2. restart

Actual Results:  
erases keywords

Expected Results:  
shouldn't
Flags: blocking-firefox3.6?
Definitely confirmed!
This should be under Component "Search", search@firefox.bugs

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.6) Gecko/20091215 Ubuntu/9.10 (karmic) Firefox/3.5.6

This happened to me twice, first updating from Firefox 3.0 to 3.5.5 then from 3.5.5 to 3.5.6, it also places the default search engines in the bottom of the list of search engines so I have to go into Manage Search Engines and move them up again (and add their keywords again).

Reproducible: Always

Steps to Reproduce:
1. update firefox
2. restart

Actual Results:
Default search engines are placed at the bottom of the list in the drop-down menu of the search bar, and their keywords (if any) are removed.
--> Toolkit::Application Update

Not a blocker, adding to the potential point release list, though. Gavin, Rob: can you take a peek and see if this is something that we can fix?
Status: UNCONFIRMED → NEW
Component: Keyboard Navigation → Application Update
Ever confirmed: true
Flags: blocking-firefox3.6?
Product: Firefox → Toolkit
QA Contact: keyboard.navigation → application.update
Whiteboard: [3.6.x]
In Launchpad I commented that I couldn't verify this on an upstream build but I could on an Ubuntu build.
I suspect the cause is that awhile back the mars were made to always remove the search engines in the install dir and install the ones included in the update. I'm not sure what the requirement is that drove that decision but I personally don't think we should be removing / adding the search engines everytime.

gavin, bsmedberg: can you shed some light on why this was done and if this behavior can be removed? If not, then this will likely need to be fixed in the search service.
Ubuntu builds don't use mar files AFAIK, just replace the package.
Right you are... over to search
Component: Application Update → Search
Product: Toolkit → Firefox
QA Contact: application.update → search
Version: unspecified → 3.5 Branch
Sounds like the same underlying issue as bug 427028. AFAICT, Ubuntu updates change the on-disk location of search engine description files, and the search service doesn't deal with having its files moved out from under it. In this case, search keywords and other metadata are associated with the path to the file on disk when that file isn't in the profile or appdir... I'm not sure why that's the case for Ubuntu's shipped builds.

Would be nice to figure out what the exact issue is, but I don't know anything about how search plugins are shipped in Ubuntu builds.
Summary: Updates overwrite/erases defined keywords for default search engines → [ubuntu] updates overwrite/erases defined keywords for default search engines
I guess this is also tracked at https://bugs.edge.launchpad.net/ubuntu/+source/firefox-3.0/+bug/338785 , but I don't really see any information there about what the actual problems are.
(In reply to comment #8)
> I guess this is also tracked at
> https://bugs.edge.launchpad.net/ubuntu/+source/firefox-3.0/+bug/338785 , but I
> don't really see any information there about what the actual problems are.

That bug is for issues of having to restart Firefox before being insured stable usage.  This is about a setting being reverted after restarting.
(In reply to comment #7)
> Sounds like the same underlying issue as bug 427028. AFAICT, Ubuntu updates
> change the on-disk location of search engine description files, and the search
> service doesn't deal with having its files moved out from under it. In this
> case, search keywords and other metadata are associated with the path to the
> file on disk when that file isn't in the profile or appdir... I'm not sure why
> that's the case for Ubuntu's shipped builds.
> 
> Would be nice to figure out what the exact issue is, but I don't know anything
> about how search plugins are shipped in Ubuntu builds.

Search engine plugins are symlink'd from APP_DIR/distribution/searchplugins -> /usr/lib/firefox-addons/searchplugins
So in reality, the search plugin location doesn't change, but the symlink location does (as the APP_DIR changes). 
For some reason, the whole path including the app dir is being stored.
For a Mozilla build, it's stored in firefox/searchplugins/foo.xml and is stored in the DB as [app]/foo.xml
For an Ubuntu build, it's stored in /usr/lib/firefox-3.5.6/distribution/searchplugins/en-US/foo.xml and stored in the DB as /usr/lib/firefox-3.5.6/distribution/searchplugins/en-US/foo.xml
Thanks for the info! I think the problem is that the search service treats distribution/searchplugins as an "unknown" location (same thing it does for extensions) since it isn't equal to the appdir searchplugins folder exactly, so it falls back to full paths.
(In reply to comment #11)
> Thanks for the info! I think the problem is that the search service treats
> distribution/searchplugins as an "unknown" location (same thing it does for
> extensions) since it isn't equal to the appdir searchplugins folder exactly, so
> it falls back to full paths.

Does anyone besides Ubuntu use distribution/searchplugins?
Our "partner repack" builds do (including http://byob.mozilla.com/ generated builds, I think).

Per discussion with asac on IRC, I think we want to just work around this by calling normalize() on the file objects we get in _loadEnginesFromDir().
We've been carrying a distro patch for a long time to do the normalize() in _loadEnginesFromDir(), and I'd like to be able to drop this at some point. Whilst normalize() works for us, it probably wouldn't work in all cases (ie, if the application directory can move around and nobody symlinked the distribution folder to a stable path).

Would you accept something like the attached patch (which basically treats distribution/searchplugins as a known location)?
(In reply to Chris Coulson from comment #14)
> Whilst normalize() works for us, it probably wouldn't work in all cases (ie,
> if the application directory can move around and nobody symlinked the
> distribution folder to a stable path).

As a simple interim solution for what is kind of an edge case, this doesn't seem so bad.

> Would you accept something like the attached patch (which basically treats
> distribution/searchplugins as a known location)?

Maybe, though I would prefer just taking the normalize() patch, I think.
(In reply to Chris Coulson from comment #14)
> Created attachment 602368 [details] [diff] [review]
> Treat distribution/searchplugins as a known location and don't use the full
> path for the search engine ID's
> 
> We've been carrying a distro patch for a long time to do the normalize() in
> _loadEnginesFromDir(), and I'd like to be able to drop this at some point.
> Whilst normalize() works for us, it probably wouldn't work in all cases (ie,
> if the application directory can move around and nobody symlinked the
> distribution folder to a stable path).
> 
> Would you accept something like the attached patch (which basically treats
> distribution/searchplugins as a known location)?

Interestingly, I have a somehow similar patch in debian, but i treat distribution/searchplugins as [app]/ instead of [distribution]/, because i moved all the default searchplugins there. And I have some additional code to make the transition smoother, although there are apparently some edge cases where that doesn't work quite well.

It is unclear if this is still an issue or not, but seeing as there haven't been complaints in recent years, I will assume this has either been worked around or fixed. So for now I'll resolve it.

If this is still an issue, I'm happy to talk about options to resolve it.

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: