mozilla removed all the code to store user-defined search engines to disk. not just disabled or changed with something else: they completely removed all the code to do this. so much about giving control to the user, yeah. and UXP inherited this broken code from the original platform. the attached patch restores this functionality. while no browser code directly rely onto it, some extensions (like "add to search bar", and some other search engine managers) either directly, or indirectly rely on that code to be there (i.e. they expect search service to save changes to disk for 'em). while "new" search service is still completely fubared (thx again, mozilla!), at least it will have some API extensions expect to be there.

in case you wonder: user-defined search engine definitions declared "obsolete" in mozilla code. read-only support for 'em is not removed, but not much more. i can't even imagine what could go wrong with your nice walled garden, mozilla...

Maybe you don't realize this but for Pale Moon 28 we did restore this code (very shortly before release in fact, Tobin did). It's just not organized the same way because both search engine back-ends are in our tree now.See Issue #699 (UXP)

Improving Mozilla code: You know you're on the right track with code changes when you spend the majority of your time deleting code.

"If you want to build a better world for yourself, you have to be willing to build one for everybody." -- Coyote Osborne

still, Pale Moon doesn't have that code in there. while "original" search service is still there, it seems that it is not used for now.

i understand that simply swapping the new code with the old code is not a way to do any changes (all the code should be carefully checked to not break other parts of the app), so i provided this patch as a partial/temporary solution, until you finish restoring the working search service. this patch simply adds two methods no other browser code is using (or even know of) for now, so it cannot break anything (i hope ;-). it is for people who want something working right now, while we are waiting for a proper solution.

eh, i should make my descriptions clearer, so i don't have to write second wall of text explaining my intents. ;-)

p.s.: i just got mad that Pale Moon reverts my edited search engine parameters after each shutdown and restart, so i looked for a reason for it to do so, and found this hilarious mozilla "improvement". and yeah, i found the old core you're working on (that is where i took those methods from! ;-). never wanted to say that PM team is ignorant about this change, and isn't working on restoring things to a sane state. bad wording from my side. sorry.

Last edited by ketmar on Wed, 05 Sep 2018, 07:43, edited 1 time in total.

Your patch is for toolkit/components/search/current/ which is not used in Pale Moon (Pale Moon uses toolkit/components/search/original/ !). Your patch would have no influence on Pale Moon, just on other applications that default to toolkit/components/search/current/

Improving Mozilla code: You know you're on the right track with code changes when you spend the majority of your time deleting code.

"If you want to build a better world for yourself, you have to be willing to build one for everybody." -- Coyote Osborne

ahem. it looks that i really broke my build files then. 'cause i have a patched search engine editor UI, and it is unable to save changes to disk with current Pale Moon builds...

p.s.: AH! i see now. the code is in there, indeed, but "batch task" is calling only `_buildCache()`, which is not calling `_seriazileToFile()`. so the only required fix is to put `if (!engine._readOnly && engine._file) engine._serializeToFile()` somewhere arond line 3061 of "current/nsSearchService.js".

sorry for the noise. please, close this topic then.

Last edited by ketmar on Wed, 05 Sep 2018, 07:56, edited 1 time in total.

that is what you got for trying to post when you're really out of life energy. i meant "orginal/nsSearchService.js", of course, silly me. there, in original, it still calls `_buildCache()` as batch task, and still doesn't bother to update disk files, never calling `engine._serializeToFile()`. not a big deal for default search engine editor ('cause it can be barely called "editor" at all), but my extended editor allows to change much more things (see pix), and they're lost on update/restart, of course.

normally, i'm doing slightly better job on checking things, but i tried to post this in a hurry (to make sure it doesn't lost before i'll have to move away from computer for some time -- otherwise i'd surely forget about it), and messed it all. sorry again.

Attachments

Last edited by ketmar on Wed, 05 Sep 2018, 08:44, edited 1 time in total.

Crippled how? I tested every bit on the functionality.. It both reads and writes searchplugins to the proper location in the profile just like Tycho. Diffing the files between Tycho and 44 were virtually identical.

From what I see of his work he is extending functionality beyond what the browser did as implimented.

I am not opposed to enhancing it but every exposed bit of functionality is working between Tycho and 28. Install both from trigger and metatag advertising, reorder, remove, remove default, restore default, read and write to/from profile directory, search suggestions, associated prefs.. I checked it all.

the thing is that the usual "browser-search-engine-modified"/"engine-changed" signal, which did delayed save of changed engine parameters, doesn't save anything (to "%PROFILE%/searchplugins") anymore. this affects every extension that expects search service to lazily save changes by this signal (as it always does). my modified search engine editor is just the easiest way (for me) to trigger this, but i bet that it is not the only thing that rely on saving changes to disk by this signal. after all, i found such code in one of "search engines management" extensions, and it worked in at least PM26. i posted screenshot to show that i can change alot of search engine params, and have a way to check if they are survived restart.

so no, *writing* to profile directory by using the "standard" signal that was used for that before is not working.

p.s.: there was also `_lazySerializeToFile()` undocumented method, that was used by "organise search engines", for example. it is removed now. that was the function that "batch task" was executing instead of cache update, and it simply serializes everything.

p.p.s.: as for why it is hard-patched into browser code, and not made into extension -- 'cause the only button i want to modify in standard dialog is "Edit engine...", but there is no way to do that without replacing the whole dialog. it doesn't matter if it is hard-patched, or made into extension, though. the standard code is using "browser-search-engine-modified"/"engine-changed" signal to commit changes, and it only updates a cache.

Last edited by ketmar on Wed, 05 Sep 2018, 14:56, edited 1 time in total.

Oh i see now, so your change will make all those extensions (including ones that failed even in Tycho) work properly again. More of a fix to a long standing mozilla induced regression. Sounds very desirable to me. I am all for it.

yeah, exactly that: mozilla broke it, and as usual with mozilla code -- it is subtle, and hard to find. it *looks* like it is working, but is stealthly broken in some unexpected way. and it is easy to miss even if you will triple-check everything, as it raises no errors, and looks fully functional. mozilla is great in such code breakage. ;-)

p.s.: to fully fix the regression, `_lazySerializeToFile()` should be restored too. i'll try to take a look to it later, as i have to check old PM code to make sure that it does what it should. but basically, it should be this:

and batch task (`_buildCache()`) should call it to write the data. this way, 'cause it is possible to monkey-patch that method, and some extensions does that, expecting `_lazySerializeToFile()` to be called.

as i said, i'll try to make a proper patch later.

Last edited by ketmar on Wed, 05 Sep 2018, 15:28, edited 2 times in total.