(partially) restoring crippled search service functionality

Talk about code development, features, specific bugs, enhancements, patches, and similar things.
Forum rules
Please keep everything here strictly on-topic.
This board is meant for Pale Moon source code development related subjects only like code snippets, patches, specific bugs, git, the repositories, etc.

This is not for tech support! Please do not post tech support questions in the "Development" board!
Please make sure not to use this board for support questions. Please post issues with specific websites, extensions, etc. in the relevant boards for those topics.

Please keep things on-topic as this forum will be used for reference for Pale Moon development. Expect topics that aren't relevant as such to be moved or deleted.
User avatar
ketmar
Lunatic
Lunatic
Posts: 369
Joined: 2015-07-28, 11:10
Location: Earth

(partially) restoring crippled search service functionality

Unread post by ketmar » 2018-09-05, 07:22

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...
Attachments
0001-shitzilla-removed-all-the-code-to-permanently-store-.patch
(8.01 KiB) Downloaded 17 times
Last edited by ketmar on 2018-09-05, 07:23, edited 1 time in total.

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 35474
Joined: 2011-08-28, 17:27
Location: Motala, SE
Contact:

Re: (partially) restoring crippled search service functionality

Unread post by Moonchild » 2018-09-05, 07:27

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)
"Sometimes, the best way to get what you want is to be a good person." -- Louis Rossmann
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite

User avatar
ketmar
Lunatic
Lunatic
Posts: 369
Joined: 2015-07-28, 11:10
Location: Earth

Re: (partially) restoring crippled search service functionality

Unread post by ketmar » 2018-09-05, 07:36

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 2018-09-05, 07:43, edited 1 time in total.

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 35474
Joined: 2011-08-28, 17:27
Location: Motala, SE
Contact:

Re: (partially) restoring crippled search service functionality

Unread post by Moonchild » 2018-09-05, 07:46

I'm not entirely sure we are on the same page here.

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/
"Sometimes, the best way to get what you want is to be a good person." -- Louis Rossmann
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite

User avatar
ketmar
Lunatic
Lunatic
Posts: 369
Joined: 2015-07-28, 11:10
Location: Earth

Re: (partially) restoring crippled search service functionality

Unread post by ketmar » 2018-09-05, 07:51

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 2018-09-05, 07:56, edited 1 time in total.

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 35474
Joined: 2011-08-28, 17:27
Location: Motala, SE
Contact:

Re: (partially) restoring crippled search service functionality

Unread post by Moonchild » 2018-09-05, 08:18

I still don't understand. toolkit/components/search/current/ should -not- be used at all in current Pale Moon builds, so any patch to it would have no effect.

See moz.build:

Code: Select all

if CONFIG['MC_PALEMOON']:
    DIRS += ['orginal']
else:
    DIRS += ['current'] 
"Sometimes, the best way to get what you want is to be a good person." -- Louis Rossmann
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite

User avatar
ketmar
Lunatic
Lunatic
Posts: 369
Joined: 2015-07-28, 11:10
Location: Earth

Re: (partially) restoring crippled search service functionality

Unread post by ketmar » 2018-09-05, 08:42

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
2018_09_05_11_38_03_815x416.png
Last edited by ketmar on 2018-09-05, 08:44, edited 1 time in total.

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 35474
Joined: 2011-08-28, 17:27
Location: Motala, SE
Contact:

Re: (partially) restoring crippled search service functionality

Unread post by Moonchild » 2018-09-05, 08:56

No worries! Seems like the adjusted Gecko/44 version we included was already crippled then! I'll patch it up.
Last edited by Moonchild on 2018-09-05, 09:24, edited 1 time in total.
"Sometimes, the best way to get what you want is to be a good person." -- Louis Rossmann
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite

New Tobin Paradigm

Re: (partially) restoring crippled search service functionality

Unread post by New Tobin Paradigm » 2018-09-05, 14:14

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.
Last edited by New Tobin Paradigm on 2018-09-05, 14:17, edited 1 time in total.

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 35474
Joined: 2011-08-28, 17:27
Location: Motala, SE
Contact:

Re: (partially) restoring crippled search service functionality

Unread post by Moonchild » 2018-09-05, 14:19

I was under the impression from Ketmar's report that any changes made to search engines from within the browser would not survive a session. Isn't that what Tycho did?
"Sometimes, the best way to get what you want is to be a good person." -- Louis Rossmann
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite

New Tobin Paradigm

Re: (partially) restoring crippled search service functionality

Unread post by New Tobin Paradigm » 2018-09-05, 14:27

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.

I don't understand exactly what the deal is.
Last edited by New Tobin Paradigm on 2018-09-05, 14:29, edited 1 time in total.

User avatar
ketmar
Lunatic
Lunatic
Posts: 369
Joined: 2015-07-28, 11:10
Location: Earth

Re: (partially) restoring crippled search service functionality

Unread post by ketmar » 2018-09-05, 14:56

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 2018-09-05, 14:56, edited 1 time in total.

New Tobin Paradigm

Re: (partially) restoring crippled search service functionality

Unread post by New Tobin Paradigm » 2018-09-05, 15:06

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.
Last edited by New Tobin Paradigm on 2018-09-05, 15:13, edited 5 times in total.

User avatar
ketmar
Lunatic
Lunatic
Posts: 369
Joined: 2015-07-28, 11:10
Location: Earth

Re: (partially) restoring crippled search service functionality

Unread post by ketmar » 2018-09-05, 15:18

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:

Code: Select all

_lazySerializeToFile: function ES_lazySerializeToFile () {
  for (let name in this._engines) {
    let engine = this._engines[name];
    if (!engine._readOnly && engine._file) {
      engine._serializeToFile();
    }
},
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 2018-09-05, 15:28, edited 2 times in total.

User avatar
ketmar
Lunatic
Lunatic
Posts: 369
Joined: 2015-07-28, 11:10
Location: Earth

Re: (partially) restoring crippled search service functionality

Unread post by ketmar » 2018-09-05, 17:08

i sent a proper patch to MC, that properly fixes this regression.

Locked