No information about overlapped or cropped data is given in Excel sheets #3040

Closed
nvaccessAuto opened this Issue Mar 4, 2013 · 36 comments

1 participant

@nvaccessAuto

Reported by sumandogra on 2013-03-04 11:14
Excel 2010
No information is given on reaching a cell that has overlapped/cropped information. This is very vital information required by the user to make the correct adjustment of the data.
Steps to reproduce:
1. Open a sheet that has data overlapped and cropped in few of the cells.
2. Navigate to such cells.
3. NVDA provides no information about such kind of data that is overlapping or cropped.

@nvaccessAuto

Comment 1 by jteh on 2013-03-04 11:54
Please attach an example and provide information about which cells should exhibit which behaviour.

@nvaccessAuto

Attachment example sheet for cropped-overlapped data.xlsx added by sumandogra on 2013-03-04 12:38
Description:
an example sheet with data cropped in second column and data overlapped in 3rd column.

@nvaccessAuto

Comment 2 by sumandogra (in reply to comment 1) on 2013-03-04 12:41
attachment made.
Replying to jteh:

Please attach an example and provide information about which cells should exhibit which behaviour.

@nvaccessAuto

Comment 3 by siddhartha_iitd on 2014-12-01 05:34
These questions have been answered on the NVDA mailing list. I'm listing the questions and the corresponding answers here for ease of tracking:

'' I've found a solution,using vba, to determine whether some text in a

'' cell in MS Excel is overflowing to the adjacent cell or is being cropped

'' due to lesser width of cell than the text. Now for implementation of

'' this solution, a few more problems need to be solved:

Great! For future reference, it's probably best that these kind of
questions are asked on the ticket so that we can keep track of
everything associated with a single ticket. Obviously, more general or
elaborate discussions need to be had on the list, but I think specific
implementation questions like this are best on the ticket. Thanks.

'' 1) Where exactly should this solution be integrated in the code ? [about a method determineTextCroppedOverlapped() in excel.py to find this

'' information and then to have it called by speakObjectProperties() in

'' speech.py](How

'')

I don't think we need a new concept for this, as it fits quite nicely
into the obscured state and perhaps the recently introduced idea of
locationText. This is how we handle similar issues in PowerPoint.

'' 3) Do we want this information to be spoken all the time or should the

'' option of whether to provide this information be given to the user (As

'' is the case with JAWS) ? If we choose the latter, we can assign a

'' key-stroke to switch between the two modes or there might be better

'' solutions to this.

The obscured state would always be reported. The user can then query for
more info with NVDA+numpadDelete.

@nvaccessAuto

Comment 4 by siddhartha_iitd on 2014-12-04 14:32
Is there any way we can calculate the text width of text present in a cell ? I've already tried pasting text into a cell in an empty column and then, doing an autofit on that column and fetching the column width. But, this is not an elegant solution for many reasons. The other solution available is to create a LUT(look-up table) containing character-width for all the fonts and then using this table to calculate the width for entire text. Please suggest an efficient way to solve this issue. Thanks.

@nvaccessAuto

Comment 6 by siddhartha_iitd (in reply to comment 3) on 2014-12-17 10:37

I don't think we need a new concept for this, as it fits quite nicely

into the obscured state and perhaps the recently introduced idea of

locationText. This is how we handle similar issues in PowerPoint.

The obscured state would always be reported. The user can then query for

more info with NVDA+numpadDelete.

If the text in a cell extends to the adjacent cell(s), those adjacent cells should have STATE_OBSCURED. But, what about the current cell ? It should say something like 'text overlapping to adjacent cells'.

@nvaccessAuto

Comment 9 by siddhartha_iitd on 2015-01-30 11:00
Please find the code-fix for this issue at branch in_t3040 at following url:
https://bitbucket.org/manish_agrawal/nvda

@nvaccessAuto

Comment 11 by mdcurran on 2015-02-02 02:19
Some code review:

  • There are a lot of commented out log calls which should be removed.
  • There are several whitespace issues (lines just containing tabs, tabs at the end of lines, spaces at the beginning of lines before tabs).
  • I think you should create a temporary DC with CreateCompatibleDC, passing it the DC you got from GetDC. So you're not affecting the Window's actual DC. Use this new DC in your cals to SelectObject. However, if there is a particular reason why you did not do it this way, let me know.
  • You need to restore the old bitmap and font objects you replaced with the SelectObject calls with another SelectObject call passing the old object returned from the first SelectObject call once you are done with the new objects (before calling DeleteObject). Following is a rough example of the pattern:
tempBitmap=CreateCompatibleBitmap(dc)
oldBitmap=SelectObject(dc,tempBitmap)
# do stuff with temp Bitmap
SelectObject(dc,oldBitmap)
DeleteObject(tempBitmap)
@nvaccessAuto

Comment 12 by siddhartha_iitd on 2015-02-02 10:14
Thanks for code review!
I've implemented the suggestions regarding CreateCompatibleDC, restoring old bitmap and font objects.
Removed the commented log statements.
I've tried to remove the whitespace issues wherever I could find them. I've removed the tabs at end of lines and lines just containing tabs from excel.py. But, if some of these issues still exist please let me know where in NVDA code should I look for them.

The updated code is available at branch in_t3040 at following url:
https://bitbucket.org/manish_agrawal/nvda

@nvaccessAuto

Comment 13 by mdcurran on 2015-02-03 23:48
Some more code review:

  • You need to delete the DC you created with CreateCompatibleDC by calling DeleteDC once you are done with it.
  • tempBmp is the old bitmap. I'd rename it to hOldBitmap to stop confusion. Also you are accidentally deleting it, instead of deleting hBitmap (the one you created).
  • There is no need to save the return values from things like DeleteObject and SelectObject (when replacing at the end). It is just a waste of variables.
  • Rename STATE_OVERLAPPING to STATE_OBSCURING.
@nvaccessAuto

Comment 14 by siddhartha_iitd on 2015-02-04 07:23
Thanks!
Made the changes and uploaded the code at:
https://bitbucket.org/manish_agrawal/nvda

@nvaccessAuto

Comment 15 by mdcurran on 2015-02-04 23:01
A few more things:

  • Delete tempDC with DeleteDC, not DeleteObject.
  • You still have quite a few whitespace issues. Especially spaces at the beginning of lines before tabs. Please read your entire diff with
git diff master...
@nvaccessAuto

Comment 16 by siddhartha_iitd on 2015-02-06 09:56
Thanks!
Made the changes. Updated code is available in branch in_t3040 at following url:
https://bitbucket.org/manish_agrawal/nvda

@nvaccessAuto

Comment 17 by mdcurran (in reply to comment 16) on 2015-02-06 17:11
Replying to siddhartha_iitd:
Your last commit is bdceae3cb67bbdfacbb3d575c5e74c74ed304bd8.
This was the one where you renamed overlapping to obscuring. Perhaps you forgot to push?

@nvaccessAuto

Comment 18 by siddhartha_iitd on 2015-02-06 18:27
Pushed the changes now...

@nvaccessAuto

Comment 19 by Michael Curran <mick@... on 2015-02-07 05:56
In [c4b2ec8]:
```CommitTicketReference repository="" revision="c4b2ec8ad6316e2a65c1002e89d272e4e564a122"
Merge branch 't3040' into next. Incubates #3040

Changes:
Added labels: incubating
@nvaccessAuto

Comment 20 by mdcurran on 2015-02-07 06:03
Thanks. I've now taken your work and placed it in NV Access's t3040 branch. I however made the following two changes:

  • You were releasing the wrong DC. When you get a DC with GetDC it must be released (but not deleted) with ReleaseDC. When you create a DC with CreateCompatibleDC it must be deleted (but not released) with DeleteDC. I moved this to just below CreatecompatibleDC as you no longer need hDC after this point.
  • I removed the _overlapText and location text code. This was incomplete and totally broken, which meant fetching location info with NVDA+numpadDelete just threw an exception. Perhaps we can think about implementing this at a later stage.

You removed pretty much all of your whitespace errors which is great. thank you.

I have now put your code on next for incubation, and hopeful inclusion in master in a few weeks.

@nvaccessAuto

Comment 21 by siddhartha_iitd on 2015-02-08 19:33
Thank you very much Mike for your help.

@nvaccessAuto

Comment 22 by leonarddr (in reply to comment 19) on 2015-02-10 15:35
Replying to Michael Curran :

In [```

#!CommitTicketReference repository="" revision="c4b2ec8ad6316e2a65c1002e89d272e4e564a122"

Merge branch 't3040' into next. Incubates #3040

I'm afraid this creates problems for me. I'm getting the following error since this changeset, c4b2ec8~ doesn't produce the error.
``ERROR - eventHandler.executeEvent (16:32:23):
error executing event: gainFocus on with extra args of {}
Traceback (most recent call last):
File "eventHandler.pyc", line 143, in executeEvent
File "eventHandler.pyc", line 91, in init
File "eventHandler.pyc", line 98, in next
File "NVDAObjects__init__.pyc", line 862, in event_gainFocus
File "NVDAObjects__init__.pyc", line 806, in reportFocus
File "speech.pyc", line 284, in speakObject
File "baseObject.pyc", line 34, in get
File "baseObject.pyc", line 110, in getPropertyViaCache
File "NVDAObjects\window\excel.pyc", line 565, in get_states
File "baseObject.pyc", line 34, in __get

File "baseObject.pyc", line 110, in _getPropertyViaCache
File "NVDAObjects\window\excel.pyc", line 643, in _get__overlapInfo
File "NVDAObjects\window\excel.pyc", line 585, in getCellWidthAndTextWidth
TypeError: int() argument must be a string or a number, not 'NoneType'
DEBUGWARNING - IAccessibleHandler.accessibleObjectFromEvent (16:32:23):
oleacc.AccessibleObjectFromEvent with window 9700810, objectID 254 and childID 0: [Error -2147467259](c4b2ec8]:

) Unspecified error

DEBUGWARNING - IAccessibleHandler.accessibleObjectFromEvent (16:32:23):
oleacc.AccessibleObjectFromEvent with window 9700810, objectID 255 and childID 0: -2147467259 Unspecified error``
The cell which creates the error contains '' 1633309'

@nvaccessAuto

Comment 23 by siddhartha_iitd on 2015-02-12 09:17
Is there any way I can reproduce the error ?
In method getCellWidthAndTextWidth int() is used at only place to convert FontSize returned by excelObject.Font.Size to integer. I checked the return value of int(Font.Size) on blank cells, Date items, alphnumeric text, etc. Still I couldn't reproduce the error.
Thanks!

@nvaccessAuto

Attachment t3040_bug.xlsx added by leonarddr on 2015-02-12 09:30
Description:
Excel sheet to reproduce bug popping up after changes in this ticket

@nvaccessAuto

Comment 24 by leonarddr (in reply to comment 23) on 2015-02-12 09:31
Replying to siddhartha_iitd:

Is there any way I can reproduce the error ?

In method getCellWidthAndTextWidth int() is used at only place to convert FontSize returned by excelObject.Font.Size to integer. I checked the return value of int(Font.Size) on blank cells, Date items, alphnumeric text, etc. Still I couldn't reproduce the error.

Thanks!

Added an attachment for reproducing.

@nvaccessAuto

Comment 25 by siddhartha_iitd on 2015-02-12 10:29
Thanks!
Without using NVDA, in the attachment, when I move to the cells in first column (column 'A'), excel shows an error mark, which says that 'The number in this cell is formatted as text or preceded by an apostrophe'. This is the reason why excelObject.Font.Size is unable to get the size data and is set to None.
One change that can be done to prevent this is to explicitly set the fontsize to default value 11 in such cases when font size is set to None.

@nvaccessAuto

Comment 26 by leonarddr (in reply to comment 25) on 2015-02-12 21:07
Replying to siddhartha_iitd:

One change that can be done to prevent this is to explicitly set the fontsize to default value 11 in such cases when font size is set to None.

How about a value of 0?

@nvaccessAuto

Comment 27 by siddhartha_iitd on 2015-02-13 04:52
0 font size implies absence of text in a cell. Empty cells are already handled correctly by this implementation. Though it could be any value, the excel default font size value is 11.

@nvaccessAuto

Comment 28 by siddhartha_iitd on 2015-02-17 08:52
Required change: set iFontSize to 11 if excelCellObject.Font.Size is None is made. The updated code is available in branch in_t3040 at following url:
​​https://bitbucket.org/manish_agrawal/nvda

@nvaccessAuto

Comment 29 by siddhartha_iitd on 2015-02-27 09:38
Added STATE_CROPPED and STATE_OVERFLOWING states in controlTypes.py. Now, using these states in excel.
The changes are available in branch in_t3040_new at following url:
​​​https://bitbucket.org/manish_agrawal/nvda

@nvaccessAuto

Comment 30 by mdcurran on 2015-03-16 05:10
Thanks for the fix. However 1 query:
Why have you changed width from width+5 to width? what does this do?
Also, there is a log statement that should not be there.

@nvaccessAuto

Comment 31 by mdcurran on 2015-03-18 02:08
on the call I thought you said you addressed my last comments. I don't think you pushed your changes yet?

@nvaccessAuto

Comment 32 by siddhartha_iitd on 2015-03-20 07:34
I'm so sorry Mike. I thought I already pushed the changes. The updated code is available in branch in_t3040_new at following url:
https://bitbucket.org/manish_agrawal/nvda

@nvaccessAuto

Comment 33 by Michael Curran <mick@... on 2015-03-31 00:19
In [2e2b33d]:
```CommitTicketReference repository="" revision="2e2b33d2b034b835ff9f721bc63f316caec95f52"
Merge branch 't3040' into next. Incubates #3040

@nvaccessAuto

Comment 34 by siddhartha_iitd on 2015-03-31 04:31
Thank You!

@nvaccessAuto

Comment 35 by James Teh <jamie@... on 2015-04-14 06:38
In [785fc4e]:
```CommitTicketReference repository="" revision="785fc4ec2700c444ac3360a92384edc8d84654b6"
In Microsoft Excel, NVDA now reports when a cell has overflowing or cropped content.

Fixes #3040.

Changes:
Removed labels: incubating
State: closed
@nvaccessAuto

Comment 36 by jteh on 2015-04-14 06:41
Changes:
Milestone changed from None to 2015.2

@nvaccessAuto

Comment 37 by surfer0627 on 2015-04-14 09:06
Thank you developers for your work. This is an useful feature.
I'm wondering that adding a carriage return to a cell will affect the result or not?
STR:
1. Open Excel.
2. Type string "_" (8 times) in cell a1
NVDA shows: overflowing
3. Press Alt+enter (add a carriage return)
NVDA shows: nothing
4. Delete the carriage return
NVDA shows: nothing, too.

@nvaccessAuto

Comment 38 by surfer0627 on 2015-05-21 08:34
Hi,
Here is a braille issue:
Do we need quote marks for messages after the content?
"overflowing" "cropped content"
It helps users distinguish message from content.
Thanks.

@nvaccessAuto nvaccessAuto added this to the 2015.2 milestone Nov 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment