sc/qa/unit/tiledrendering/data/DocumentWithLongFirstColumn.ods |binary
 sc/qa/unit/tiledrendering/tiledrendering.cxx                   |   86 
++++++++++
 sc/source/ui/unoobj/docuno.cxx                                 |    7 
 sc/source/ui/view/gridwin.cxx                                  |    2 
 sc/source/ui/view/viewdata.cxx                                 |    4 
 5 files changed, 96 insertions(+), 3 deletions(-)

New commits:
commit d815fa555d3717c76d961ee964c41929de36dc03
Author:     Tomaž Vajngerl <tomaz.vajng...@collabora.co.uk>
AuthorDate: Thu Jan 11 15:08:21 2024 +0900
Commit:     Andras Timar <andras.ti...@collabora.com>
CommitDate: Tue Jan 16 23:43:17 2024 +0100

    sc lok: set the GridWindow size to the client area size + test
    
    This solves the issue when we click on a long first column (size
    800px - default GridWindow size) at a right end position (>800px)
    and the selection jumped to the neighbouring cell. This solution
    reverts the workaround for this issue and properly sets the
    GridWindow to the current reported client visible area size (set
    with the ScModelObj::setClientVisibleArea call).
    
    Also includes a test for this issue.
    
    Change-Id: Ia5449893a90ffa0072aad68d772ad9b00206f113
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/161907
    Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com>
    Reviewed-by: Miklos Vajna <vmik...@collabora.com>
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/161957
    Tested-by: Jenkins
    Reviewed-by: Tomaž Vajngerl <qui...@gmail.com>

diff --git a/sc/qa/unit/tiledrendering/data/DocumentWithLongFirstColumn.ods 
b/sc/qa/unit/tiledrendering/data/DocumentWithLongFirstColumn.ods
new file mode 100644
index 000000000000..27fc3f45c543
Binary files /dev/null and 
b/sc/qa/unit/tiledrendering/data/DocumentWithLongFirstColumn.ods differ
diff --git a/sc/qa/unit/tiledrendering/tiledrendering.cxx 
b/sc/qa/unit/tiledrendering/tiledrendering.cxx
index ee2b50522e3f..f59635769343 100644
--- a/sc/qa/unit/tiledrendering/tiledrendering.cxx
+++ b/sc/qa/unit/tiledrendering/tiledrendering.cxx
@@ -3349,6 +3349,92 @@ CPPUNIT_TEST_FIXTURE(ScTiledRenderingTest, 
testSidebarLocale)
     CPPUNIT_ASSERT_EQUAL(std::string("de-DE"), aLocale);
 }
 
+CPPUNIT_TEST_FIXTURE(ScTiledRenderingTest, testLongFirstColumnMouseClick)
+{
+    // Document has a long first column. We want to mouse-click on the column 
and
+    // check the selection changed to this column.
+
+    // The issue we want to reproduce is that the click on a cell in the first 
column that is
+    // very long (longer than ~800px default size of GridWindow) triggers a 
code-path where the cell
+    // selected is the neighbouring cell even when we clicked on the area of 
the first cell.
+
+    ScModelObj* pModelObj = createDoc("DocumentWithLongFirstColumn.ods");
+    CPPUNIT_ASSERT(pModelObj);
+    
pModelObj->initializeForTiledRendering(uno::Sequence<beans::PropertyValue>());
+
+    // Fetch current view data
+    ScViewData* pViewData = ScDocShell::GetViewData();
+    CPPUNIT_ASSERT(pViewData);
+    double nPPTX = pViewData->GetPPTX();
+    double nPPTY = pViewData->GetPPTX();
+
+    // Set click position
+
+    // Left side of the first cell
+    int leftCellSideX = 1 / nPPTX; // convert pixels to logical units
+
+    // Right side of the first cell. First cell is long so click somewhere 
more than 800px (default of GridWindow size).
+    int rightCellSideX = 1000 / nPPTX; // convert pixels to logical units
+
+    // Vettical position - doesn't matter - select the first row
+    int y = 1 / nPPTY;
+
+    // Setup view #1
+    ViewCallback aView1;
+    // Set client rect to 2000 x 2000 pixels
+    pModelObj->setClientVisibleArea(tools::Rectangle(0, 0, 2000 / nPPTX, 2000 
/ nPPTY));
+    Scheduler::ProcessEventsToIdle();
+
+    // Click at on the left side of A1 cell
+    pModelObj->postMouseEvent(LOK_MOUSEEVENT_MOUSEBUTTONDOWN, leftCellSideX, 
y, /*count=*/ 1, /*buttons=*/ 1, /*modifier=*/0);
+    pModelObj->postMouseEvent(LOK_MOUSEEVENT_MOUSEBUTTONUP, leftCellSideX, y, 
/*count=*/ 1, /*buttons=*/ 1, /*modifier=*/0);
+    Scheduler::ProcessEventsToIdle();
+
+    // Check the A1 cell is selected in view #1
+    CPPUNIT_ASSERT_EQUAL(SCCOL(0), ScDocShell::GetViewData()->GetCurX());
+    CPPUNIT_ASSERT_EQUAL(SCROW(0), ScDocShell::GetViewData()->GetCurY());
+
+    // Click at on the right side of A1 cell
+    pModelObj->postMouseEvent(LOK_MOUSEEVENT_MOUSEBUTTONDOWN, rightCellSideX, 
y, /*count=*/ 1, /*buttons=*/ 1, /*modifier=*/0);
+    pModelObj->postMouseEvent(LOK_MOUSEEVENT_MOUSEBUTTONUP, rightCellSideX, y, 
/*count=*/ 1, /*buttons=*/ 1, /*modifier=*/0);
+    Scheduler::ProcessEventsToIdle();
+
+    // Check the A1 cell is selected in view #1
+    CPPUNIT_ASSERT_EQUAL(SCCOL(0), ScDocShell::GetViewData()->GetCurX());
+    CPPUNIT_ASSERT_EQUAL(SCROW(0), ScDocShell::GetViewData()->GetCurY());
+
+    // Try to check the same scenario in a new view
+
+    // Setup view #2
+    SfxLokHelper::createView();
+    int nView2 = SfxLokHelper::getView();
+    ViewCallback aView2;
+    // Set client rect to 2000 x 2000 pixels
+    pModelObj->setClientVisibleArea(tools::Rectangle(0, 0, 2000 / nPPTX, 2000 
/ nPPTY));
+
+    // Lets make sure we are in view #2
+    SfxLokHelper::setView(nView2);
+    Scheduler::ProcessEventsToIdle();
+
+    // Click at on the left side of A1 cell
+    pModelObj->postMouseEvent(LOK_MOUSEEVENT_MOUSEBUTTONDOWN, leftCellSideX, 
y, /*count=*/ 1, /*buttons=*/ 1, /*modifier=*/0);
+    pModelObj->postMouseEvent(LOK_MOUSEEVENT_MOUSEBUTTONUP, leftCellSideX, y, 
/*count=*/ 1, /*buttons=*/ 1, /*modifier=*/0);
+    Scheduler::ProcessEventsToIdle();
+
+    // Check the A1 cell is selected in view #2
+    CPPUNIT_ASSERT_EQUAL(SCCOL(0), ScDocShell::GetViewData()->GetCurX());
+    CPPUNIT_ASSERT_EQUAL(SCROW(0), ScDocShell::GetViewData()->GetCurY());
+
+    // Click at on the right side of A1 cell
+    pModelObj->postMouseEvent(LOK_MOUSEEVENT_MOUSEBUTTONDOWN, rightCellSideX, 
y, /*count=*/ 1, /*buttons=*/ 1, /*modifier=*/0);
+    pModelObj->postMouseEvent(LOK_MOUSEEVENT_MOUSEBUTTONUP, rightCellSideX, y, 
/*count=*/ 1, /*buttons=*/ 1, /*modifier=*/0);
+    Scheduler::ProcessEventsToIdle();
+
+    // Check the A1 cell is selected in view #2
+    CPPUNIT_ASSERT_EQUAL(SCCOL(0), ScDocShell::GetViewData()->GetCurX());
+    CPPUNIT_ASSERT_EQUAL(SCROW(0), ScDocShell::GetViewData()->GetCurY());
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sc/source/ui/unoobj/docuno.cxx b/sc/source/ui/unoobj/docuno.cxx
index 8b72acd44c8c..da265c97a1b0 100644
--- a/sc/source/ui/unoobj/docuno.cxx
+++ b/sc/source/ui/unoobj/docuno.cxx
@@ -1156,6 +1156,13 @@ void ScModelObj::setClientVisibleArea(const 
tools::Rectangle& rRectangle)
         if (pTabView)
             pTabView->extendTiledAreaIfNeeded();
     }
+
+    // Set the GridWindow size to the client area size, so that the logic in 
GridWindow works correctly
+    // for the current view and doesn't cause any unexpected behaviour related 
to window size and checks if we are
+    // outside of the window.
+
+    ScGridWindow* pGridWindow = pViewData->GetActiveWin();
+    pGridWindow->SetOutputSizePixel(Size(rRectangle.GetWidth() * 
pViewData->GetPPTX(), rRectangle.GetHeight() * pViewData->GetPPTY()));
 }
 
 void ScModelObj::setOutlineState(bool bColumn, int nLevel, int nIndex, bool 
bHidden)
diff --git a/sc/source/ui/view/viewdata.cxx b/sc/source/ui/view/viewdata.cxx
index d09c0a9e0aec..e09fdcd62a92 100644
--- a/sc/source/ui/view/viewdata.cxx
+++ b/sc/source/ui/view/viewdata.cxx
@@ -2851,19 +2851,15 @@ void ScViewData::GetPosFromPixel( tools::Long nClickX, 
tools::Long nClickY, ScSp
         }
     }
 
-    bool bLOK = comphelper::LibreOfficeKit::isActive();
     //  cells too big?
-    // Work-around this for LOK, because the screen size is in not set 
correctly
-    // for all views and we will geturn the wrong position in case we send a 
click
-    // that is outside the set screen grid area
-    if (rPosX == nStartPosX && nClickX > 0 && !bLOK)
+    if (rPosX == nStartPosX && nClickX > 0)
     {
          if (pView)
             aScrSize.setWidth( pView->GetGridWidth(eHWhich) );
          if ( nClickX > aScrSize.Width() )
             ++rPosX;
     }
-    if (rPosY == nStartPosY && nClickY > 0 && !bLOK)
+    if (rPosY == nStartPosY && nClickY > 0)
     {
         if (pView)
             aScrSize.setHeight( pView->GetGridHeight(eVWhich) );
commit fbddb2b5f0bee0be249fd4bb8a991eaeb3a30a92
Author:     Tomaž Vajngerl <tomaz.vajng...@collabora.co.uk>
AuthorDate: Thu Jan 11 13:25:16 2024 +0900
Commit:     Andras Timar <andras.ti...@collabora.com>
CommitDate: Tue Jan 16 23:43:01 2024 +0100

    sc: don't stop if we can't cast one view to ScTabViewShell
    
    Probably the intention here was to skip the view if we can't
    cast it to ScTabViewShell and not stop updating all the other
    views when we can't cast.
    
    Change-Id: Idbc2cd2d0c0ac5773fb45badb9098cb776d56691
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/161958
    Tested-by: Jenkins
    Reviewed-by: Tomaž Vajngerl <qui...@gmail.com>

diff --git a/sc/source/ui/view/gridwin.cxx b/sc/source/ui/view/gridwin.cxx
index 3f4f6b219c67..cdba90d8d436 100644
--- a/sc/source/ui/view/gridwin.cxx
+++ b/sc/source/ui/view/gridwin.cxx
@@ -6352,7 +6352,7 @@ void ScGridWindow::updateOtherKitSelections() const
     {
         auto pOther = dynamic_cast<const ScTabViewShell *>(it);
         if (!pOther)
-            return;
+            continue;
 
         // Fetch pixels & convert for each view separately.
         tools::Rectangle aBoundingBox;
commit 59cf022fdd81ac269b9cea377e4bb9b5211cda27
Author:     Tomaž Vajngerl <tomaz.vajng...@collabora.co.uk>
AuthorDate: Fri Jan 5 21:05:50 2024 +0900
Commit:     Andras Timar <andras.ti...@collabora.com>
CommitDate: Tue Jan 16 23:42:47 2024 +0100

    sc: LOK work-around for mouse click selecting the wrong cell
    
    Don't increase the cell selection if we detect the mouse click
    was out of the screen (grid window) area. Only do this when LOK
    is enabled.
    
    Change-Id: I97922e9d02500d7cedeaa5fa29d4ca344950ff47
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/161662
    Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com>
    Reviewed-by: Miklos Vajna <vmik...@collabora.com>
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/161816
    Tested-by: Jenkins
    Reviewed-by: Caolán McNamara <caolan.mcnam...@collabora.com>

diff --git a/sc/source/ui/view/viewdata.cxx b/sc/source/ui/view/viewdata.cxx
index 25e602669e80..d09c0a9e0aec 100644
--- a/sc/source/ui/view/viewdata.cxx
+++ b/sc/source/ui/view/viewdata.cxx
@@ -2851,15 +2851,19 @@ void ScViewData::GetPosFromPixel( tools::Long nClickX, 
tools::Long nClickY, ScSp
         }
     }
 
+    bool bLOK = comphelper::LibreOfficeKit::isActive();
     //  cells too big?
-    if ( rPosX == nStartPosX && nClickX > 0 )
+    // Work-around this for LOK, because the screen size is in not set 
correctly
+    // for all views and we will geturn the wrong position in case we send a 
click
+    // that is outside the set screen grid area
+    if (rPosX == nStartPosX && nClickX > 0 && !bLOK)
     {
          if (pView)
             aScrSize.setWidth( pView->GetGridWidth(eHWhich) );
          if ( nClickX > aScrSize.Width() )
             ++rPosX;
     }
-    if ( rPosY == nStartPosY && nClickY > 0 )
+    if (rPosY == nStartPosY && nClickY > 0 && !bLOK)
     {
         if (pView)
             aScrSize.setHeight( pView->GetGridHeight(eVWhich) );

Reply via email to