Bug Hunting Session
Bug 94679 - EDITING: Text selection with Shift+PageDown broken part2
Summary: EDITING: Text selection with Shift+PageDown broken part2
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
4.4.0.0.alpha0+ Master
Hardware: Other Linux (All)
: medium normal
Assignee: Justin L
URL:
Whiteboard: target:5.1.0 target:5.0.4 target:4.4.6
Keywords: bibisected, bisected, regression
: 70500 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-10-01 18:57 UTC by Justin L
Modified: 2016-10-25 19:17 UTC (History)
8 users (show)

See Also:
Crash report or crash signature:


Attachments
basic 4 page document allowing a few SHIFT-pagedown / pageups (26.02 KB, application/vnd.oasis.opendocument.text)
2015-10-01 18:57 UTC, Justin L
Details
two backtraces (6.19 KB, text/plain)
2015-10-03 08:02 UTC, Justin L
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Justin L 2015-10-01 18:57:29 UTC
Created attachment 119175 [details]
basic 4 page document allowing a few SHIFT-pagedown / pageups

Shift-PgDn stops selecting very easily (and randomly).  This is probably not related to bug 40550 since this particular bug was introduced on Sept 21, 2014.

It is hard to believe, but this bug was bibisected (and confirmed) to be introduced by commit e5f52eddda1230eba971881223601bb7aa255d6b
    Author:     Samuel Mehrbrodt <s.mehrbrodt@gmail.com>
    AuthorDate: Sun Sep 21 17:59:58 2014 +0200
    Commit:     Samuel Mehrbrodt <s.mehrbrodt@gmail.com>
    CommitDate: Sun Sep 21 16:01:18 2014 +0000
    
        fdo#81475 New layout for Writer standard and formatting toolbars


specifically:
 <toolbar:toolbaritem xlink:href=".uno:InsertObjectChart"/>
as well as
 <toolbar:toolbaritem xlink:href=".uno:InsertObjectStarMath"/>
now.

Setting the icon to "not show" or turning off the toolbar itself is NOT a workaround.  Removing these items from sw/uiconfig/swriter/toolbar/standardbar.xml before compiling does resolve the problem, however.

Shift-UP and Shift-DOWN did not seem to be affected.
Comment 1 Maxim Monastirsky 2015-10-01 19:41:56 UTC
Confirmed. It comes from http://opengrok.libreoffice.org/xref/core/sw/source/uibase/shells/textsh.cxx#665. Commenting this rSh.Pop() makes Shift-PgDn work as expected.
Comment 2 Justin L 2015-10-02 19:10:58 UTC
In sw/source/uibase/wrtsh.cxx  SwWrtShell::Pop() it is pretty much inevitable that *m_fnSetCrsr() will be set to SetCrsrKillSel.  I have no idea why pop-ing rSh should automatically negate any selections - especially when the whole point of pushing/popping was to check for a hiddenRange.  

bool SwWrtShell::Pop( bool bOldCrsr )
{
    bool bRet = SwCrsrShell::Pop( bOldCrsr );
// if something was pushed - meaning we successfully popped something (bRet)
// and the cursor is part of a selection
    if( bRet && IsSelection() )
    {
        m_fnSetCrsr = &SwWrtShell::SetCrsrKillSel;
        m_fnKillSel = &SwWrtShell::ResetSelect;
    }
    return bRet;
}

A PageUp/Down calls sw/source/uibase/wrtsh/move.cxx SwWrtShell::PushCrsr() which calls this *m_fnSetCrsr() and kills the selection.

SelectHiddenRange() aborts if there is a selection anyway, so if textsh.cxx wraps the push/pop in a if( !HasSelection() ) that should resolve this bug.
Comment 3 Justin L 2015-10-03 08:02:16 UTC
Created attachment 119230 [details]
two backtraces

proposed fix at https://gerrit.libreoffice.org/#/c/19108
Comment 4 Justin L 2015-10-03 08:06:10 UTC
Bug 40550 and Bug 61224 also seem to be fixed (but probably not by this patch - their fixes were most likely masked by this bug).
Comment 5 Cor Nouws 2015-10-03 09:27:31 UTC
(In reply to Justin L from comment #4)
> Bug 40550 and Bug 61224 also seem to be fixed (but probably not by this
> patch - their fixes were most likely masked by this bug).

Wonderful - Thanks! Look forward to test/work with it.
Comment 6 Michael Meeks 2015-10-03 10:54:28 UTC
Miklos - any chance of a review of this one ? Andras - if Miklos / other writer guys are happy - I'd love to have this back-ported to older LfC versions - it's a hyper-annoying bug. Justin - great work chasing this down =) !
Comment 7 Commit Notification 2015-10-09 13:58:42 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "master":

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

tdf#94679 Writer: fix lost selection with Shift-PageDown

It will be available in 5.1.0.

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 8 Commit Notification 2015-10-09 16:21:55 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "libreoffice-4-4":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=22560075218c6166d44367957f4a733ea50ff9c3&h=libreoffice-4-4

tdf#94679 Writer: fix lost selection with Shift-PageDown

It will be available in 4.4.7.

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 9 Commit Notification 2015-10-09 16:22:09 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "libreoffice-5-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=f6c542f88aaae45873ba0258df16a05a12bf5968&h=libreoffice-5-0

tdf#94679 Writer: fix lost selection with Shift-PageDown

It will be available in 5.0.4.

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 10 Commit Notification 2015-10-14 21:38:19 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "libreoffice-4-4-6":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=3051c054579b201db9a36af20f09d455fbdf7422&h=libreoffice-4-4-6

tdf#94679 Writer: fix lost selection with Shift-PageDown

It will be available in 4.4.6.

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 11 Miklos Vajna 2015-11-28 00:26:48 UTC
I assume this wasn't closed by accident.
Comment 12 Robinson Tryon (qubit) 2015-12-17 10:33:27 UTC Comment hidden (obsolete)
Comment 13 Justin L 2016-06-23 11:07:19 UTC
*** Bug 70500 has been marked as a duplicate of this bug. ***