Bug 51004 - replace mozilla address book parser with a new implementation that doesn't link to mozilla libraries
Summary: replace mozilla address book parser with a new implementation that doesn't li...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Base (show other bugs)
Version:
(earliest affected)
Master old -3.6
Hardware: Other All
: medium normal
Assignee: DavidO
QA Contact:
URL:
Whiteboard:
Keywords: difficultyInteresting, easyHack, skillCpp
Depends on:
Blocks: MozBaseKillOff
  Show dependency treegraph
 
Reported: 2012-06-12 04:32 UTC by Caolán McNamara
Modified: 2015-12-16 00:43 UTC (History)
8 users (show)

See Also:
Crash report or crash signature:


Attachments
Screenshot - do not see some non-latin symbols (11.84 KB, image/png)
2013-03-05 13:07 UTC, Nick222
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Caolán McNamara 2012-06-12 04:32:18 UTC
We haul around an ancient copy of mozilla in-tree. There are a few pieces that make use of it, but the "most annoying" piece of functionality that would be lost by removing it is the mozilla thunderbird address book integration. Which is further complicated by having to patch mozilla itself to allow it to subvert the lock that's placed on in-use addressbooks, i.e. the code doesn't work with an unpatched mozilla anyway.

We should replace our mozilla-api using code with a direct mab parser so we can finally ditch the silliness of building a full copy of mozilla.

http://www.scalingweb.com/mork_parser.php looks the most promising approach, i.e. just import that small BSD-licenced project instead, e.g. can follow the libexttextcat example

Then add the mork parser license statement into the right places e.g. 
http://opengrok.libreoffice.org/search?q=WiseGuys&project=core&defs=&refs=&path=&hist=

Then convert connectivity/source/drivers/mozab to use that instead of the mozilla xpcom apis.

(btw connectivity/source/drivers/mozab does some more things above just mozilla address book integration, e.g. finding firefox/thunderbird profile dir)

Then remove the "whether to build Mozilla addressbook connectivity" and "strangely hacked up Mozilla binary" stuff from configure.in and unwind any of the build conditionals which assume that mozilla address book integration requires the "strangely hacked up Mozilla binary"

"writer:file->wizards->address data source" and "base: connect to existing database" are two routes that can use the mozilla address book for testing purposes. (obviously, at the moment you only get it listed if you configure with --enable-mozilla --without-system-mozilla)

Some more possibilities and references:
http://www.jwz.org/doc/mailsum.html
https://bugzilla.mozilla.org/show_bug.cgi?id=424446
https://bugzilla.mozilla.org/show_bug.cgi?id=241438
https://bugzilla.mozilla.org/show_bug.cgi?id=382876
http://search.cpan.org/~kript/Mozilla-Mork-0.01/lib/Mozilla/Mork.pm
Comment 1 DavidO 2012-08-03 07:56:47 UTC
Recently i made some progress on feature/mork branch.
The follow steps are completed:

o MorkParser is integrated (some weird problems are fixed, must blog on it, is really funny ;-)
o Mozilla stuff is completely pinched off from the build.
o Minimal Profile Discovery is shamelessly stolen from mozab driver.
o mork_helper executable can discover the local profile, find the address book and dump it.
o you can already import your address book: you would see AddressBook table. If you open it, LO crashes in OCommonStatement::parseSql.

Question:
Currently i am messing around with query parser in OCommonStatement:
http://cgit.freedesktop.org/libreoffice/core/tree/connectivity/source/drivers/mork/MStatement.cxx?h=feature/mork#n79
http://cgit.freedesktop.org/libreoffice/core/tree/connectivity/source/drivers/mork/MStatement.cxx?h=feature/mork#n201

What is the best way to deal with it? Do we need here a parser at all?
Should one be written von scratch that delegate to MorkParser? Or may be i could import the whole data 
in shadow sqllite file and forward the address book queries to it? 
What would be the simplest solution here that püossible would work?
(It is not clear how XPCOM driver actually does that magic).

Still to do:
o MetaData retrieval
o Lists facility (you can define list/group in address book) and it should be imported as own table
o Query handler/parser
o ResultSet population
Comment 2 Michael Meeks 2012-08-03 11:06:18 UTC
Fridrich, weren't you following this area to try to rid us of the internal mozilla ? :-) do you have thoughts here ?
Comment 3 DavidO 2012-08-18 11:55:23 UTC
Some progress here:

default AdressBook table can already be imported on feature/mork branch.
Still to do:

import group tables
lazy loading (query take place to often now)
sorting
entry manipulation: add, delete, etc. is this actually needed?
Comment 4 Caolán McNamara 2012-08-19 19:36:37 UTC
Sounds like great progress to me. Re what we need, I'd say we need whatever the writer mail merge can use when you click through its wizard. If that doesn't make use of it then we can probably skip/defer that functionality.
Comment 5 Michael Stahl 2012-08-20 09:56:53 UTC
hmm.. i'd actually prefer not to have any editing support in there and have the driver be read-only, because we directly access the address book files from the Mozilla profile, and who knows what could happen if you have that open in a running Thunderbird at the same time and perhaps make changes there.

for the use case most people have (mail merge) read-only is sufficient.
Comment 6 Caolán McNamara 2012-08-20 11:13:16 UTC
yeah, agreed.
Comment 7 Lionel Elie Mamane 2012-09-03 05:48:09 UTC
(In reply to comment #1)
> Question:
> Currently i am messing around with query parser in OCommonStatement:
> http://cgit.freedesktop.org/libreoffice/core/tree/connectivity/source/drivers/mork/MStatement.cxx?h=feature/mork#n79
> http://cgit.freedesktop.org/libreoffice/core/tree/connectivity/source/drivers/mork/MStatement.cxx?h=feature/mork#n201
 
> What is the best way to deal with it? Do we need here a parser at all?

Well, I haven't looked at it in detail, but I assume the driver needs to parse the SQL statement to know what to return: which columns, which rows, ...

LibreOffice already contains a SQL parser: core/connectivity/source/parse/
Rather than reinvent the wheel, maybe you can reuse it?

> Or may be I could import the whole data 
> in shadow sqllite file and forward the address book queries to it?

That could be a possibility; sqlite is not necessarily a good choice, especially if the data contains dates/timestamps/... OTOH, you probably don't want to wait until we have a good Java-free embedded database engine, unless you feel like working on that, too ;-) (bug 51781 and bug 51780 and dependencies contain these discussions)

> What would be the simplest solution here that püossible would work?
> (It is not clear how XPCOM driver actually does that magic).

It seems to use its driver's m_xMSFactory member to do the parsing, but that's a ::com::sun::star::uno::Reference< ::com::sun::star::lang::XMultiServiceFactory >, so I'm confused.
Comment 8 Lionel Elie Mamane 2012-09-03 07:33:10 UTC
(In reply to comment #3)

> entry manipulation: add, delete, etc. is this actually needed?

Making it read-only sounds like a reasonable choice. In this case, please:

 - return privilege metadata indicating that read-only status (only SELECT privilege)

   this indicates to the LibO UI not to let the user attempt any modification

 - if a modification is attempted (e.g. through a macro),
   throw a SQLException with SQLSTATE set to 2F002
   and Message set to some descriptive user-friendly text;
   something like "the LibreOffice Mozilla address book driver is read-only"
   should do.
   ErrorCode is the driver's domain; just set it to a fixed unique value
   that is different from other situations in which you throw an SQLException.
Comment 9 DavidO 2012-09-13 22:27:30 UTC
Another progress here. It was not lazy loading, but infinite loop. It was really hard to find/debug, because LO grabbed the mouse and keyboard ;-)
@Michael Stahl thank you again for your help!

@Caolan, what do you mean by saying:

[...]
Re what we need, I'd say we need whatever the
writer mail merge can use when you click through its wizard. If that doesn't
make use of it then we can probably skip/defer that functionality.
[...]

I have no idea, what are you talking about.
Comment 10 DavidO 2012-09-16 22:26:25 UTC
Another progress here.

Done issues:

sorting
queries with complex expressions
error handling (exceptions forwarded to UI)

TODOs:

Group tables are ignored (only AddressBook Table is imported).

SQL column aliasing is not implemented (select FirstName as name from  AdressBook produces error).

SQL regexp query expression is not implemented.

ScalingWeb.com License is only copied into connectivity/source/drivers/mork directory.
Comment 11 Caolán McNamara 2012-09-26 08:25:11 UTC
re "@Caolan, what do you mean by saying:

[...]
Re what we need, I'd say we need whatever the
writer mail merge can use when you click through its wizard. If that doesn't
make use of it then we can probably skip/defer that functionality.
[...]"

My belief is that the biggest and more useful usecase for address book integration is mail merges, so whatever features are required to make writer->tools->mail merge wizard->next->email message->select address book-> etc. functional are the important bits and everything else is gravy
Comment 12 DavidO 2012-10-05 09:59:13 UTC
(In reply to comment #11)
> 
> My belief is that the biggest and more useful usecase for address book
> integration is mail merges, so whatever features are required to make
> writer->tools->mail merge wizard->next->email message->select address book->
> etc. functional are the important bits and everything else is gravy

mail merges/meat: it just works.

gravy: 

regexp is not implemented, if query has something like that 'where name like "foo*bar" ' error is thrown

group/list tables are not implemented. Working on it.

history.mab is not implemented (only abook.mab is processed). Would be not a big deal.

Question:

i have seen link on this page
http://wiki.documentfoundation.org/Development/Build_System/Module_status
to this issue:

"please do not convert moz, it is going to be replaced; see bug fdo#51004"

i doubt, that we should track replacement for Outlook (Express) data provider for Windows in context of this issue. Moz ist still needed to access it. 
We should probably file a new bz entry for that.
Comment 13 Nick222 2013-03-05 13:06:18 UTC
Xubuntu 12.10-64 Rus
Thunderbird 17.0.3

1) Cannot see some non-latin symbols (see screenshot in attachment).

2) Worked only with main address book ("Personal Address Book") - not with user created.
Comment 14 Nick222 2013-03-05 13:07:34 UTC
Created attachment 75955 [details]
Screenshot - do not see some non-latin symbols
Comment 15 Nick222 2013-03-05 13:09:04 UTC
(In reply to comment #13)
> Xubuntu 12.10-64 Rus
> Thunderbird 17.0.3
> 
> 1) Cannot see some non-latin symbols (see screenshot in attachment).
> 
> 2) Worked only with main address book ("Personal Address Book") - not with
> user created.

LibreOffice 4.0.1.1 (ID: 400m0(Build:1)
Comment 16 Lionel Elie Mamane 2013-03-05 13:15:18 UTC
(In reply to comment #13)
> Xubuntu 12.10-64 Rus
> Thunderbird 17.0.3
> 
> 1) Cannot see some non-latin symbols (see screenshot in attachment).
> 
> 2) Worked only with main address book ("Personal Address Book") - not with
> user created.


Please open new bugs for each of these different *new* issues / bugs instead of reopening a *different* bug, and mixing two issues in one bugzilla entry. As to non-main address book (user-created address books), did it work before?

Thanks in advance.
Comment 17 Robinson Tryon (qubit) 2015-12-16 00:43:08 UTC
Migrating Whiteboard tags to Keywords: (EasyHack SkillCpp DifficultyInteresting)
[NinjaEdit]