After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 660605 - XL overrides cell borders by next cells, gnumeric doesn't (3rd party XLS file)
XL overrides cell borders by next cells, gnumeric doesn't (3rd party XLS file)
Status: RESOLVED FIXED
Product: Gnumeric
Classification: Applications
Component: import/export MS Excel (tm)
1.11.x
Other Linux
: Normal normal
: ---
Assigned To: Jody Goldberg
Jody Goldberg
Depends on:
Blocks:
 
 
Reported: 2011-10-01 02:05 UTC by Valek Filippov
Modified: 2011-10-07 18:58 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Bottom border of row 26 cells looks thick in XL (set to 2 by top border of row 27 cells) (28.00 KB, application/msexcel)
2011-10-01 02:05 UTC, Valek Filippov
  Details
Cell D7 is "all borders = 1", cell D8 is "top border = 2", "1C Enterprise" way of saving borders (25.00 KB, application/msexcel)
2011-10-01 02:06 UTC, Valek Filippov
  Details
Same as in comment #1 but made by XL2k7 (25.00 KB, application/msexcel)
2011-10-01 02:07 UTC, Valek Filippov
  Details
proposed patch (4.33 KB, patch)
2011-10-06 18:09 UTC, Andreas J. Guelzow
committed Details | Review
Crafted XLS with all 'border vs border' combinations (27.50 KB, application/msexcel)
2011-10-07 12:14 UTC, Valek Filippov
  Details
screenshot of the "borders.xls" file opened in xl2k7 (10.31 KB, image/png)
2011-10-07 12:15 UTC, Valek Filippov
  Details

Description Valek Filippov 2011-10-01 02:05:41 UTC
Created attachment 197932 [details]
Bottom border of row 26 cells looks thick in XL (set to 2 by top border of row 27 cells)

"1C" generates XLS files with 'shared' cell borders set to non-0 for both cells.
XL overrides border of the 1st cell by border of adjacent cell.

XL itself does not save files this way. One of the adjacent border is set to 0 always.
Comment 1 Valek Filippov 2011-10-01 02:06:51 UTC
Created attachment 197933 [details]
Cell D7 is "all borders = 1", cell D8 is "top border = 2", "1C Enterprise" way of saving borders
Comment 2 Valek Filippov 2011-10-01 02:07:38 UTC
Created attachment 197934 [details]
Same as in comment #1 but made by XL2k7
Comment 3 Valek Filippov 2011-10-01 02:08:29 UTC
For files in comments #1 and #2 borders are in XF records 0x3e and 0x3f.
Comment 4 Andreas J. Guelzow 2011-10-01 03:07:25 UTC
what would happen if the cells are switched, ie. 1st cell: "border shared with second cell = 2" and 2nd cell: "all borders = 1". What would XL do?
Comment 5 Valek Filippov 2011-10-01 03:33:20 UTC
It draws think one.
I will do more tests with single/double lines borders and colors.
Comment 6 Valek Filippov 2011-10-01 13:23:52 UTC
d7		d8			result
1 (thin)	2 (medium)		2 (medium)
2 (medium)	1 (thin)		2 (medium)
2 (medium)	5 (thick)		5 (thick)
2 (medium)	3 (dashed)		2 (medium)
3 (dashed)	2 (medium)		2 (medium)
6 (double)	2 (medium)		6 (double)
2 (medium)	8 (med-dash)		2 (medium)
8 (med-dash)	2 (medium)		2 (medium)
6 (double)	5 (thick)		6 (double)
2 (medium)	7 (hair)		2 (medium)
7 (hair)	2 (medium)		2 (medium)
5 (thick)	6 (double)		6 (double)

double > thick > medium > thin > hair
solid > dashed

--
I didn't check all combinations, can do if required.
Do you want test files?
Comment 7 Valek Filippov 2011-10-01 14:09:43 UTC
One important case was missed.
1 (thin) 8 (med-dash) -> 8 (med-dash).
Comment 8 Andreas J. Guelzow 2011-10-04 19:36:19 UTC
Note that we are in fact importing the complete border information. If you load the file  of comment #1, select D8 format->cell->format then the top border is in fact recognized as medium.

The issue is that we read the information and don't check for inconsistency. Gnumeric will always display the border of the cell to the top or left.

In a way the situation is there fore worse. We are creating an inconsistent border setup when importing those files.
Comment 9 Andreas J. Guelzow 2011-10-06 05:52:56 UTC
Valek,

so with
	GNM_STYLE_BORDER_NONE			= 0x0,
	GNM_STYLE_BORDER_THIN			= 0x1,
	GNM_STYLE_BORDER_MEDIUM			= 0x2,
	GNM_STYLE_BORDER_DASHED			= 0x3,
	GNM_STYLE_BORDER_DOTTED			= 0x4,
	GNM_STYLE_BORDER_THICK			= 0x5,
	GNM_STYLE_BORDER_DOUBLE			= 0x6,
	GNM_STYLE_BORDER_HAIR			= 0x7,
	GNM_STYLE_BORDER_MEDIUM_DASH		= 0x8,
	GNM_STYLE_BORDER_DASH_DOT			= 0x9,
	GNM_STYLE_BORDER_MEDIUM_DASH_DOT		= 0xa,
	GNM_STYLE_BORDER_DASH_DOT_DOT		= 0xb,
	GNM_STYLE_BORDER_MEDIUM_DASH_DOT_DOT	= 0xc,
	GNM_STYLE_BORDER_SLANTED_DASH_DOT		= 0xd,
what is the preference ordering:

	GNM_STYLE_BORDER_DOUBLE			= 0x6,
	GNM_STYLE_BORDER_THICK			= 0x5,
	GNM_STYLE_BORDER_MEDIUM			= 0x2,
	GNM_STYLE_BORDER_MEDIUM_DASH		= 0x8,
	GNM_STYLE_BORDER_THIN			= 0x1,
	GNM_STYLE_BORDER_HAIR			= 0x7,

where do the other ones fit in?
GNM_STYLE_BORDER_DASHED should be below GNM_STYLE_BORDER_MEDIUM but the above does not provide enough info to determine where it belongs...
Comment 10 Andreas J. Guelzow 2011-10-06 18:09:27 UTC
Created attachment 198458 [details] [review]
proposed patch

Valek,

please test this patch. (The choice table in excel_choose_border still needs work to address the, to me unknown, preferences between the borders.)
Comment 11 Valek Filippov 2011-10-07 12:13:08 UTC
Andreas,

Based on my latest tests prefs are:
double > thick > medium > medium-dash > medium-dash-dot > slanted dash-dot > medium dash-dot-dot > thin > dashed > dotted > dash-dot > dash-dot-dot > hair

So the table could be like this:
-------------
		= { { 0,0,0,0,0,0,0,0,0,0,0,0,0,0 },  /* GNM_STYLE_BORDER_NONE */
		    { 1,0,0,1,1,0,0,1,0,1,0,1,0,0 },  /* GNM_STYLE_BORDER_THIN */
		    { 1,1,0,1,1,0,0,1,1,1,1,1,1,1 },  /* GNM_STYLE_BORDER_MEDIUM */
		    { 1,0,0,0,1,0,0,1,0,1,0,1,0,0 },  /* GNM_STYLE_BORDER_DASHED */
		    { 1,0,0,0,0,0,0,1,0,1,0,1,0,0 },  /* GNM_STYLE_BORDER_DOTTED */
		    { 1,1,1,1,1,0,0,1,1,1,1,1,1,1 },  /* GNM_STYLE_BORDER_THICK */
		    { 1,1,1,1,1,1,0,1,1,1,1,1,1,1 },  /* GNM_STYLE_BORDER_DOUBLE */
		    { 1,0,0,0,0,0,0,0,0,0,0,0,0,0 },  /* GNM_STYLE_BORDER_HAIR */
		    { 1,1,0,1,1,0,0,1,0,1,1,1,1,1 },  /* GNM_STYLE_BORDER_MEDIUM_DASH */
		    { 1,0,0,0,0,0,0,1,0,0,0,1,0,0 },  /* GNM_STYLE_BORDER_DASH_DOT */
		    { 1,1,0,1,1,0,0,1,0,1,0,1,1,1 },  /* GNM_STYLE_BORDER_MEDIUM_DASH_DOT */
		    { 1,0,0,0,0,0,0,1,0,0,0,0,0,0 },  /* GNM_STYLE_BORDER_DASH_DOT_DOT */
		    { 1,1,0,1,1,0,0,1,0,1,0,1,0,0 },  /* GNM_STYLE_BORDER_MEDIUM_DASH_DOT_DOT */
		    { 1,1,0,1,1,0,0,1,0,1,0,1,1,0 }   /* GNM_STYLE_BORDER_SLANTED_DASH_DOT */

------------

You patch (with s/gnm__border/gnm_border/) fixes borders in attached files, but not in one I've made to test all combinations at once. (Will attach right after this comment).
Comment 12 Valek Filippov 2011-10-07 12:14:48 UTC
Created attachment 198522 [details]
Crafted XLS with all 'border vs border' combinations
Comment 13 Valek Filippov 2011-10-07 12:15:26 UTC
Created attachment 198523 [details]
screenshot of the "borders.xls" file opened in xl2k7
Comment 14 Andreas J. Guelzow 2011-10-07 17:52:35 UTC
As expected, the file of comment #12 does not trigger the code of the patch. XL2k7 does not create inconsistent borders. So the problem shown with that screenshot is a different issue.
Comment 15 Andreas J. Guelzow 2011-10-07 17:56:57 UTC
Comment on attachment 198458 [details] [review]
proposed patch

committed with an adjusted choice table
Comment 16 Andreas J. Guelzow 2011-10-07 17:59:22 UTC
The initial problem is fixed.

This bug report remains open for the border issue identified by the file in comment #12 (and matching screen shot of comment #13)
Comment 17 Andreas J. Guelzow 2011-10-07 18:13:21 UTC
Valek, would you be able to provide a file with incorrect borders where only 2 cells have borders set. (Debugging that would be much less confusing.) Thanks
Comment 18 Andreas J. Guelzow 2011-10-07 18:42:51 UTC
I think I misunderstood the issue.

I assume no that the file of comment #12 was also generated via 1C. 

The code path for this file does not use excel_set_xf but excel_set_xf_segment. In this case our original code just drops the second border spec rather than creating an inconsistent border situation. We need to provide the overwrite here too.

I also noticed that the earlier fix assumes that the cell information is read from top left (which may always be true in the excel_set_xf case). This is clearly not the case in the excel_set_xf_segment case.
Comment 19 Andreas J. Guelzow 2011-10-07 18:58:35 UTC
Okay, the file of comment #12 was handcrafted. Unless we encounter a file from the wild with xf-segments that are in conflict, this is not really a bug worthwhile addressing.