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 | 8 5 files changed, 96 insertions(+), 7 deletions(-)
New commits: commit da9f8f9f93a1146f90c21ceec143a827063e24f5 Author: Tomaž Vajngerl <[email protected]> AuthorDate: Thu Jan 11 13:25:16 2024 +0900 Commit: Tomaž Vajngerl <[email protected]> CommitDate: Mon Jan 15 10:04:00 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 <[email protected]> diff --git a/sc/source/ui/view/gridwin.cxx b/sc/source/ui/view/gridwin.cxx index 446badbb1244..186243702985 100644 --- a/sc/source/ui/view/gridwin.cxx +++ b/sc/source/ui/view/gridwin.cxx @@ -6375,7 +6375,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 a4e429f18cc61b091ee5f0e85e2786b7dd22ccd3 Author: Tomaž Vajngerl <[email protected]> AuthorDate: Thu Jan 11 15:08:21 2024 +0900 Commit: Tomaž Vajngerl <[email protected]> CommitDate: Mon Jan 15 10:03:54 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 <[email protected]> Reviewed-by: Miklos Vajna <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/161957 Tested-by: Jenkins Reviewed-by: Tomaž Vajngerl <[email protected]> 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 902fee6293ef..f31eff1bd001 100644 --- a/sc/qa/unit/tiledrendering/tiledrendering.cxx +++ b/sc/qa/unit/tiledrendering/tiledrendering.cxx @@ -3277,6 +3277,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 4b412e4f54c1..a7f0a5a7674f 100644 --- a/sc/source/ui/unoobj/docuno.cxx +++ b/sc/source/ui/unoobj/docuno.cxx @@ -1157,6 +1157,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) );
