Bug 62937 - incorrect/no progression from Database Wizard step 1
Summary: incorrect/no progression from Database Wizard step 1
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Base (show other bugs)
Version:
(earliest affected)
4.1.0.0.alpha0+ Master
Hardware: Other All
: high critical
Assignee: Mathias Hasselmann
URL:
Whiteboard: target:4.1.0 target:4.0.5
Keywords: regression
Depends on:
Blocks: mab4.1 64447
  Show dependency treegraph
 
Reported: 2013-03-30 15:04 UTC by Terrence Enger
Modified: 2013-07-01 13:16 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Terrence Enger 2013-03-30 15:04:07 UTC
To reproduce ...

(1) Run the LibreOffice Database Wizard by issuing the shell command
    `soffice --norestore --base`.

    Program displays window Database Wizard Step 1 "Select database"
    with "What do you wnat to do?" defaulted to "Create a new
    database".

(2) Click next.

    Expected program action: to display Database Wizard step 2 "Save
    and proceed".

    Actual program action: to display Database Wizard step 2 "Set up a
    connection to a JDBC database


I see this with a lightly hacked master commit af17e2a, pulled around
2013-03-29 11:45 UTC, configured with

    --enable-option-checking
    --enable-dbgutil
    --enable-crashdump
    --disable-build-mozilla
    --without-system-postgresql
    --without-myspell-dicts
    --without-help
    --with-extra-buildid

built and running on ubuntu-natty (11.04) 32-bit

    $ uname -a
    Linux cougar-natty 2.6.38-16-generic #67-Ubuntu SMP Thu Sep 6 18:00:43 UTC 2012 i686 athlon i386 GNU/Linux

    $ gcc --version
    gcc (Ubuntu/Linaro 4.5.2-8ubuntu4) 4.5.2
    Copyright (C) 2010 Free Software Foundation, Inc.
    This is free software; see the source for copying conditions.  There is NO
    warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

    $ java -version
    java version "1.6.0_24"
    OpenJDK Runtime Environment (IcedTea6 1.11.5) (6b24-1.11.5-0ubuntu1~11.04.1)
    OpenJDK Client VM (build 20.0-b12, mixed mode, sharing)

My "light hack" is the one quoted in
<https://bugs.freedesktop.org/show_bug.cgi?id=61688#c13>.


For comparison, ...

(*) This problem is not apparent in similarly hacked commit 51a18f9
    from 2013-03-14 or in release candidate 4.0.2.2.

(*) This problem is not apparent if in Step 1 you click explicitly on
    the defaulted "Create a new database"


Terry.
Comment 1 Terrence Enger 2013-04-03 00:05:12 UTC
Additionally, if in wizard step 1 you select "Connect to existing database ... JDBC", the <Next> button does not advance to step 2 at all.

Petr,

I notice that commit f3f512, 2013-03-21, which you reviewed, changed a lot in this area.  Can you give me any help?

I cannot cc Mathias in the bug report; shall try to email him in the normal way.

Terry.
Comment 2 Mathias Hasselmann 2013-04-03 08:23:23 UTC
maybe even related to this merge request? must check later.
https://gerrit.libreoffice.org/#/c/2889/
Comment 3 Julien Nabet 2013-04-06 13:52:57 UTC
Mathias: in http://opengrok.libreoffice.org/xref/core/dbaccess/source/ui/dlg/generalpage.cxx#190, I noticed these lines:

    196         const OUString eOldSelection = m_eCurrentSelection;
    197 
    198         m_pDatasourceType->SelectEntry( getDatasourceName( _rSet ) );
    199 
    200         // notify our listener that our type selection has changed (if so)
    201         if ( eOldSelection != m_eCurrentSelection )
    202         {
    203             setParentTitle( m_eCurrentSelection );
    204             onTypeSelected( m_eCurrentSelection );
    205         }

For the moment, it seems impossible to enter in the if block.
Comment 4 Julien Nabet 2013-04-07 08:46:33 UTC
Mathias: any update for this one?
Comment 5 Julien Nabet 2013-05-09 09:16:41 UTC
Mathias: I updated my git repo and still have the problem, have you found the problem?
Comment 6 Julien Nabet 2013-05-09 10:06:15 UTC
Increase importance since it's a regression.
Comment 7 Julien Nabet 2013-05-09 12:51:08 UTC
Mathias: I don't know if you received previous notification but put you in cc just in case.
Comment 8 Lionel Elie Mamane 2013-05-11 14:07:45 UTC
*** Bug 64447 has been marked as a duplicate of this bug. ***
Comment 9 Petr Mladek 2013-05-13 09:31:36 UTC
Mathias, will you have time to look at this anytime soon, please?

We are going to create 4.1.0.0.beta1 the following week and this bug might block the Base testing.
Comment 10 Alex Thurgood 2013-05-13 10:09:09 UTC
On my master build on Mac OSX :

Version: 4.1.0.0.alpha0+
Build ID: efc4761686b9c566d9a45fb9ab3984d7704603b

clicking on Next in the wizard takes me to step 2 where I have to choose a JDBC connection ????

The first screen of the database connection wizard only lists two steps.

Confirming bug on OSX.

Alex
Comment 11 Commit Notification 2013-05-21 09:05:33 UTC
Petr Mladek committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=13df1b95e83a46e847311b49d4c14260cac682af

allow to create new database using the wizard again (fdo#62937)



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 12 Petr Mladek 2013-05-21 09:20:02 UTC
It really helped to call the dead code as found by Julien in the comment #3. It looked like optimization, so I simply removed the "if". It works now => closing the bug.

BTW: The merge request https://gerrit.libreoffice.org/#/c/2889/ did not help. It seems to be unrelated.

Mathias: I t would be great if you could review the change and provide a better fix that keeps the optimization. I think that you know the code better than anyone else at this stage because you did the split.
Comment 13 Julien Nabet 2013-06-04 20:59:34 UTC
Just taking a new look to this one and this that this function may help if it would return something (dbaccess/source/ui/dlg/generalpage.cxx):
    309     void OGeneralPage::implSetCurrentType( const OUString& _eType )
    310     {
    311         if ( _eType == m_eCurrentSelection )
    312             return;
    313 
    314         m_eCurrentSelection = _eType;
    315     }
could be:
    309     boolean OGeneralPage::implSetCurrentType( const OUString& _eType )
    310     {
    311         if ( _eType == m_eCurrentSelection )
    312             return false; // no setting since nothing changed
    313 
    314         m_eCurrentSelection = _eType;
    315         return true;
    316     }


Of course, all the callers should be updated and particularly this one (also in dbaccess/source/ui/dlg/generalpage.cxx):
    180     void OGeneralPage::onTypeSelected(const OUString& _sURLPrefix)
    181     {
    182         // the new URL text as indicated by the selection history
    183         implSetCurrentType( _sURLPrefix );
    184 
    185         switchMessage(_sURLPrefix);
    186 
    187         if ( m_aTypeSelectHandler.IsSet() )
    188             m_aTypeSelectHandler.Call(this);
    189     }
This specific method is called in the part I noticed in comment 3.

Petr/Lionel: do you think it may worth it to dig this way? (or perhaps you had new from Mathias)
Comment 14 Lionel Elie Mamane 2013-06-05 04:40:01 UTC
(In reply to comment #13)
> Just taking a new look to this one and this that this function may help if
> it would return something (dbaccess/source/ui/dlg/generalpage.cxx):
>     309     void OGeneralPage::implSetCurrentType( const OUString& _eType )
> could be:
>     309     boolean OGeneralPage::implSetCurrentType( const OUString& _eType
> )
> This specific method is called in the part I noticed in comment 3.

> Petr/Lionel: do you think it may worth it to dig this way? (or perhaps you
> had new from Mathias)

Petr said he fixed this bug? If you want to reintroduce the optimisation (without readding the bug), sure go ahead and do so in master; just make sure to try out things :)
Comment 15 Julien Nabet 2013-06-05 06:00:29 UTC
Lionel: obviously after having searched and posted comment 3, my goal is not to reintroduce the bug :-). It was just to have some opinion to find a way to improve the fix but more thinking about this, this tracker is not the better place to do this. Moreover as you indicated, there's still fdo#64447.
In brief sorry for the noise.
Comment 16 Petr Mladek 2013-06-05 10:06:32 UTC
Julien, if you could improve the code, it would be great. I agree with Lionel that it is better to do so in master and it needs some testing.

My "fix" was kind of quick workaround. I did not understand the whole code. The change looked relatively safe and helped. Unfortunately, I will not have time to dig into the code to provide a better fix anytime soon. Please, feel free to continue.
Comment 17 Julien Nabet 2013-06-05 10:14:44 UTC
Petr: don't worry, I didn't want to push a patch directly on a branch :-)
I just tried to dig a little to find some way. Now I'm not sure at all to succeed in understanding the whole mechanism and would prefer Mathias to give a look. I don't know him but I suppose he's very busy these days.
Anyway if I change anything, it will be on master and I'll use gerrit review of course.
Meanwhile, it's great that Lionel fixed fdo#64447! (and I wouldn't like to break his fix too !:-))
Comment 18 Julien Nabet 2013-06-06 21:34:37 UTC
After having transformed implSetCurrentType so it returns a bool, just the fact to add a test on the result of implSetCurrentType in method onTypeSelect makes the bug appear again.
Far too complicate for me :-(
Comment 19 Commit Notification 2013-07-01 13:16:51 UTC
Petr Mladek committed a patch related to this issue.
It has been pushed to "libreoffice-4-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=40d76398f2be1f6efb11f79865c3b8adc3186f24&h=libreoffice-4-0

allow to create new database using the wizard again (fdo#62937)


It will be available in LibreOffice 4.0.5.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.