Bug Hunting Session
Bug 53920 - EDITING: Data Validity Cell Range not being applied to multiple selected cells
Summary: EDITING: Data Validity Cell Range not being applied to multiple selected cells
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
3.5.6.2 release
Hardware: Other All
: low normal
Assignee: Laurent BP
URL:
Whiteboard: target:4.4.0 target:4.3.3 target:4.2....
Keywords: difficultyBeginner, easyHack, skillCpp, skillDebug
Depends on:
Blocks:
 
Reported: 2012-08-22 08:19 UTC by tim
Modified: 2018-07-24 07:48 UTC (History)
6 users (show)

See Also:
Crash report or crash signature:


Attachments
A sheet trying combinations of named/unnamed references to same/other sheet. (9.15 KB, application/vnd.oasis.opendocument.spreadsheet)
2012-08-24 12:25 UTC, tim
Details

Note You need to log in before you can comment on or make changes to this bug.
Description tim 2012-08-22 08:19:29 UTC
Problem description: When applying data validation defined by a cell range to multiple selected cells, it is only applied to the first cell with the thick border.

Steps to reproduce:
1. Provide the desired values in a range of cells
2. Select the cells to be validated
3. Apply Data > Validity ... > Cell Range

Current behavior: Only the first cell validates data.

Expected behavior: All selected cells validate data.

Platform (if different from the browser): 
              
Browser: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:14.0) Gecko/20100101 Firefox/14.0.1
Comment 1 Rainer Bielefeld Retired 2012-08-23 20:30:05 UTC
[Reproducible] with "LibreOffice 3.6.1.2  German UI/Locale [Build-ID:  e29a214] on German WIN7 Home Premium (64bit), I will do some further research tomorrow
Comment 2 tim 2012-08-23 23:02:37 UTC
The problem does not appear when accessing the cell range via a named range.
Comment 3 Rainer Bielefeld Retired 2012-08-24 11:34:45 UTC
I currently only can reproduce the problem for criteria in diffenret sheet, works fine with criteria in the same sheet.

@Reporter:
can you please attach a sample document with some examples "works / works not"?
Comment 4 tim 2012-08-24 12:25:20 UTC
Created attachment 66064 [details]
A sheet trying combinations of named/unnamed references to same/other sheet.
Comment 5 Joel Madero 2012-10-15 00:15:39 UTC
Also confirming. Marking as NEW and prioritizing.

Normal - Could make it more difficult to create professional quality documents. Raised from minor to normal because it's not consistent (as Rainer has pointed out, if the validity check values are on the same sheet, there is no problem), if a user didn't know this they could end up going round and round trying to figure out how to apply the validity check to a large range of cells or worse, do the validity check one by one

Low - Not a ton of users use validity checks. Only happens under specific circumstances

@Rainer - if you think it deserves different priority, feel free to changes

@Bug Reporter - thanks for reporting this one
Comment 6 Markus Mohrhard 2012-11-21 22:09:34 UTC
code pointers:

ScValidationData is the class that contains the validation information.
Comment 7 Armin C 2013-07-23 11:36:03 UTC
Still present in Linux in 4.0.4.2 (Build ID: 9e9821abd0ffdbc09cd8c52eaa574fa09eb08f2) and 4.1.0.1 rc
Comment 8 Björn Michaelsen 2013-10-04 18:46:31 UTC
adding LibreOffice developer list as CC to unresolved EasyHacks for better visibility.

see e.g. http://nabble.documentfoundation.org/minutes-of-ESC-call-td4076214.html for details
Comment 9 Doug Naphas 2013-11-04 18:38:17 UTC
I would like to attempt this, but I am new to LibreOffice and want to have a better understanding of the code base before assigning this to myself.  Can anyone offer some pointers on getting started?
Comment 10 Markus Mohrhard 2013-11-04 18:48:24 UTC
(In reply to comment #9)
> I would like to attempt this, but I am new to LibreOffice and want to have a
> better understanding of the code base before assigning this to myself.  Can
> anyone offer some pointers on getting started?

so most of the calc specific code is inside sc/

sc is split into 3 parts:

sc/source/core contain mostly the model
sc/source/filter containing mostly the calc specific parts of the filters
sc/source/ui containing the ui related code + some other non model parts

ScValidationData is the main class for the validation feature.

Cell ranges are represented by

ScAddress a single cell address
ScRange a continuous cell range
ScRangeList a list of ScRange
ScMarkData also able to contain multi-selection

Please ask more detailed question if you need more details.

You can use opengrok.libreoffice.org for browsing the Libreoffice source code.
Comment 11 Joel Madero 2013-11-04 18:50:30 UTC
also don't forget that you can jump into the dev IRC channel or email the list to get help :)
Comment 12 Laurent BP 2014-09-12 20:31:11 UTC
Hi,

The bug seems to come from the fact that visible tab is not the initial tab. Some easy workarounds:
- don't use the picker:
   - either use a named reference (as mentioned previously)
   - or type the reference
- after using the picker, click on tab where the cells to be validated are
- apply twice Data > Validity: second time the dialog is pre-filled, you just need to click on OK

It is quite strange because after executing dialog, tab is forced to go back to initial one. See http://opengrok.libreoffice.org/xref/core/sc/source/ui/view/cellsh2.cxx#849
849  short nResult = pDlg->Execute();
850  //When picking Cell Range, other Tab may be switched. Need restore the correct tab
851  pTabViewShell->SetTabNo( nTab );

nTab value is correct. Why it is not enough? What else can be done?
Comment 13 Laurent BP 2014-09-27 06:32:04 UTC
I tried to debug what is happening at closure time after OK click.

in sc/source/ui/view/cellsh2.cxx:928
 pTabViewShell->SetValidation( aData ); 
should copy ValidationData to all selected cells. 
But in sc/source/ui/view/viewfunc.cxx:1092
function ScViewFunc::ApplySelectionPattern(
creates aFuncMark with
 ScMarkData aFuncMark( rViewData.GetMarkData() );
aFuncMark.bMarked should be true but is set to false if visible tab is not where selected cells are (in debugger, forcing it to true avoid the bug).

I tried to force to apply tab modification after sc/source/ui/view/cellsh2.cxx#851
 pTabViewShell->SetTabNo( nTab );
with pTabViewShell->PaintExtras(); but it has no effect.

So I propose to avoid the bug by just switching back tab earlier: at RefButton shrink box closure. In commit 11560, 
https://gerrit.libreoffice.org/#/c/11660/
I propose to:
- switching back tab in ScTPValidationValue::RefInputDonePostHdl 
- remove unnecessary switch back tab in cellsh2.cxx
Comment 14 Commit Notification 2014-09-30 13:53:09 UTC
Laurent Balland-Poirier committed a patch related to this issue.
It has been pushed to "master":

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

fdo#53920 Switch back tab at RefButton closure time



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 15 Commit Notification 2014-09-30 14:01:39 UTC
Laurent Balland-Poirier committed a patch related to this issue.
It has been pushed to "libreoffice-4-3":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=50eac342603ca08d808f53dc9a32bb9d1dfba372&h=libreoffice-4-3

fdo#53920 Switch back tab at RefButton closure time


It will be available in LibreOffice 4.3.3.

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 16 Commit Notification 2014-09-30 14:03:55 UTC
Laurent Balland-Poirier committed a patch related to this issue.
It has been pushed to "libreoffice-4-2":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=8cdb25a38530319e0b08d97d2706ff019797fe08&h=libreoffice-4-2

fdo#53920 Switch back tab at RefButton closure time


It will be available in LibreOffice 4.2.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 17 Kohei Yoshida 2014-09-30 19:11:45 UTC
I'll mark this fixed.
Comment 18 Robinson Tryon (qubit) 2015-12-15 12:17:24 UTC
Migrating Whiteboard tags to Keywords: (EasyHack DifficultyBeginner SkillCpp SkillDebug )
[NinjaEdit]
Comment 19 Robinson Tryon (qubit) 2016-02-18 16:37:10 UTC
Remove LibreOffice Dev List from CC on EasyHacks
(curtailing excessive email to list)
[NinjaEdit]
Comment 20 Commit Notification 2018-07-24 07:48:06 UTC
Zdeněk Crhonek committed a patch related to this issue.
It has been pushed to "master":

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

uitest for bug tdf#53920

It will be available in 6.2.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.