Adding 2.26.1 as a target for solving the Insert+f issue with paragraph-style.

(In reply to comment #1)
> Adding 2.26.1 as a target for solving the Insert+f issue with paragraph-style.

The issue with not announcing paragraph-style is the following code in scripts/apps/soffice/script.py and the other code that works with it:

...
        # The default set of text attributes to speak to the user. The
        # only difference over the default set in settings.py is to add
        # in "left-margin:" and "right-margin:".

        self.enabledBrailledTextAttributes = \
          "size:; family-name:; weight:400; indent:0mm; left-margin:0mm; " \
          "right-margin:0mm; underline:none; strikethrough:none; " \
          "justification:left; style:normal;"
        self.enabledSpokenTextAttributes = \
          "size:; family-name:; weight:400; indent:0mm; left-margin:0mm; " \
          "right-margin:0mm; underline:none; strikethrough:none; " \
          "justification:left; style:normal;"
...
        self.savedEnabledBrailledTextAttributes = \
            settings.enabledBrailledTextAttributes
        settings.enabledBrailledTextAttributes = \
            self.enabledBrailledTextAttributes

        self.savedEnabledSpokenTextAttributes = \
            settings.enabledSpokenTextAttributes
        settings.enabledSpokenTextAttributes = self.enabledSpokenTextAttributes
...


What I believe is happening is that the self.enabled* stuff overrides the default settings.  So, if one changes the text attributes in the default settings, they will be overridden by this code.  If one creates app-specific settings, however, then focus_tracking_presenter.py will load these after the script is activated, causing them to override whatever the script may have set in its activate method.

So, the no-code-changes workaround is to have the user create app-specific settings for office.  Note also that a similar situation exists for the readTableCellRow setting in office and that the text attributes overriding situation also exists for Gecko.

For office, a potential code change might be to add the left and right margin attributes to the spoken text attributes instead of replacing the entire list. I haven't tried, but I think this will allow the default attributes to apply. I'm also a bit confused about why tho left and right margin stuff is in the braille attributes.  That seems kind of weird to me.

For Gecko, I'm not sure what to do since the attributes are being replaced due to Gecko using different names for text attributes.

Created attachment 131843 [details] [review]
First pass - needs work

Here's a patch that does a few things:

1) Gets rid of adding the left and right margin stuff to the braille attributes.  It seems very weird to me to have that in there -- the left and right margin stuff is likely to be present all the time, so the braille dots will end up being kind of useless.

2) Uses the "additive" technique to add in the left and right margins stuff for the office script.  This is the portion that needs the most work.  If the left and right margin stuff is already in the existing list, this addition might cause double speaking of the margin information.  A couple workarounds would be: 1) get rid of this additional stuff altogether and require the user to make app-specific customizations for office (my preference) or 2) ensure we don't add things in if they are already in the list.

3) Adds paragraph-style to the default list of spoken attributes, and puts it first.  I'm not sure about this, but the paragraph style (which will only be spoken if it is present) seems like it might be the most important thing to present when doing an Insert+f, but I might be wrong.

Created attachment 131852 [details] [review]
Patch for where am I 

Here's a patch to try to add paragraph-style to the detailed Where Am I functionality for text areas (e.g., KP_Enter 2x quickly).  The code needed additional modification to pull attributes from the default attributes as well since OOo gives us much of the needed information in default attributes.

I'm a bit puzzled about what the detailed Where Am I code is doing for text areas, though.  There's some code at the end of where_am_I.py:_getAttributesForChar that looks like this:

        # Only return attributes for the beginning of an attribute run.
        if attribStr != self._lastAttributeString:
            self._lastAttributeString = attribStr
            return attribStr
        else:
            return ""

I believe the effect this has is that doing consecutive detailed Where Am I's on the same text is going to end up with detailed information be presented the first time, but then nothing until the user moves the caret to a location where the attributes are different.  I'm guessing this was unintentional, so the patch includes a line to unset self._lastAttributeString after the where am I operation completes.

Adding [joanie] for her comments and general sanity checking.

(In reply to comment #5)
> Adding [joanie] for her comments and general sanity checking.
> 

Looking at this now. I'll respond to your questions after I take it all in. In the meantime *independent of your patches*:

1. We're not always presenting the attributes in braille at all. :-( It's weird. I'll start digging.

2. Looking at http://www.pygtk.org/pygtk2reference/atk-constants.html#atk-text-attr-constants, paragraph-style is not an Atk text attribute. The only place I've ever seen it is in OOo. Therefore, should it be included in settings.allTextAttributes at all?

3. As for the margins, we only present them if they are not 0mm. The only way I've figured out how to make them not be 0mm is to drag the little triangles on the ruler. Since we are not presenting that spatially on the display, should we by default continue to make that one of the standard enabled braille attributes?

Stay tuned...

w.r.t point 1: I'm seeing some seriously bogus offsets, like start: 154243824 end: -18131255. I see this in accerciser too. :-( Still looking.

Okay, I created a patch to deal with the bogus offsets and get braille attributes displayed (see bug 578072). Now that they work, let me look at what Will's done here. :-)

Will, on the general (not whereAmI) patch:

 enabledSpokenTextAttributes = \
-    "size:; family-name:; weight:400; indent:0; underline:none; " \
-    "strikethrough:false; justification:left; style:normal;"
+    "paragraph-style:; size:; family-name:; weight:400; indent:0; " \
+    "underline:none; strikethrough:false; justification:left; style:normal;"

As I mentioned in comment #6, I've only seen this attribute in OOo. Given that:

1. Either we don't want it here at all, OR

2. We might want to set the default value to "Default" because, at least in OOo, paragraph-style is always present, but if it's nothing special, it's "Default".

3. We leave it as you have it here, but do the additive thing to set the default value to Default in the soffice settings.

(My rationale for 2 and 3 is this: If I press Orca+F I only care about the paragraph style if it's a heading or something other than Default.)

In addition, if we've determined that paragraph-style should be presented, I would think I'd want to see it in braille as well -- as long as it's not a style of "Default".

+        #
+        self.additionalSpokenTextAttributes = \
+          "; left-margin:0mm; right-margin:0mm;"
 
-        self.enabledBrailledTextAttributes = \
-          "size:; family-name:; weight:400; indent:0mm; left-margin:0mm; " \
-          "right-margin:0mm; underline:none; strikethrough:none; " \
-          "justification:left; style:normal;"
-        self.enabledSpokenTextAttributes = \
-          "size:; family-name:; weight:400; indent:0mm; left-margin:0mm; " \
-          "right-margin:0mm; underline:none; strikethrough:none; " \
-          "justification:left; style:normal;"

Removing these has some additional side effects it seems. In particular:

1. Elsewhere, the default value of indent is 0; in OOo it's Omm. 

2. Elsewhere the default value of strikethrough is apparently false; it's None in OOo. 

Why they (and the fine folks at Mozilla) do this sort of thing is beyond me. But anyhoo.... As a result of the fact that they do and did, with your patch pressing Orca+F presents the fact that there's no indentation and no strikethrough (both of which are presumed). I suspect this is why I'm seeing pretty much all text as having attributes in braille too, but I've not dug further into that.

Beyond that, the removal of the left-margin and right-margin from the enabled braille attributes means, as I commented earlier, that Orca users relying upon braille might miss the fact that someone's repositioned the by dragging those ruler thingies around. Ultimately we should probably add that to the "visual/spatial braille presentation" to-do list. In the meantime, I'm not sure what the right answer is.

We definitely need to do *something* about how we handle text attributes in Orca (e.g. bug 558497). I'll give it some more thought.

Tried the whereAmI patch.

Given:

    This <b>is</b> a <i>silly</i> test.

(where the mark up is actually applied formatting)

Without patch detailed whereAmI:
~~~~~~~~~~~~~~
SPEECH OUTPUT: ''
SPEECH OUTPUT: 'paragraph'
SPEECH OUTPUT: 'This  ;  bold is a  ;  italic silly test.'
~~~~~~~~~~~~~~

With patch detailed whereAmI:
~~~~~~~~~~~~~~
SPEECH OUTPUT: ''
SPEECH OUTPUT: 'paragraph'
SPEECH OUTPUT: ' ;  paragraph style Default weight 400 This  ;  paragraph style Default bold is ;  paragraph style Default weight 400  a  ;  paragraph style Default italic weight 400 silly ;  paragraph style Default weight 400  test.'
~~~~~~~~~~~~~~

Thanks Joanie!

(In reply to comment #6)
> 2. Looking at
> http://www.pygtk.org/pygtk2reference/atk-constants.html#atk-text-attr-constants,
> paragraph-style is not an Atk text attribute. The only place I've ever seen it
> is in OOo. Therefore, should it be included in settings.allTextAttributes at
> all?

I can imagine other applications that provide this sort of thing offering it at some point.  Is there an advantage to not including it in settings.py?

> 3. As for the margins, we only present them if they are not 0mm. The only way
> I've figured out how to make them not be 0mm is to drag the little triangles on
> the ruler. Since we are not presenting that spatially on the display, should we
> by default continue to make that one of the standard enabled braille
> attributes?

It seems that attributes that are not clear cut "there" or "not there" type things are kind of difficult to present using raised braille dots on individual cells.  Paragraph styles are one such thing, margins are another, justification is another.  Weight is another, but we fudge that by calling anything > 400 bold.

But, we indeed are not presenting it spatially in braille, so I'm not quite sure what to do.  I'll punt and ask Mike.  :-)  I'm going to guess the answer from Mike might be to create a flashed message (bug #576847) that lists out the attributes in text form rather than relying on raising specific dots for a cell.

(In reply to comment #9)
> Will, on the general (not whereAmI) patch:
> 
>  enabledSpokenTextAttributes = \
> -    "size:; family-name:; weight:400; indent:0; underline:none; " \
> -    "strikethrough:false; justification:left; style:normal;"
> +    "paragraph-style:; size:; family-name:; weight:400; indent:0; " \
> +    "underline:none; strikethrough:false; justification:left; style:normal;"
...
> 2. We might want to set the default value to "Default" because, at least in
> OOo, paragraph-style is always present, but if it's nothing special, it's
> "Default".

"Text body" is another.  I think that paragraph styles are quite a bit different from attributes such as bold or italic.  When I edit documents, I refer to the paragraph style _all_ the time, and it's very meaningful to me to know whether the paragraph style is "Default", "Text body", "Heading 1", etc.  

In fact, I tend to rely more upon the paragraph style than the specific bold/italic/etc. attributes, with the exception of when the character style has attributes that differ from the paragraph style.  So, for example, knowing something is *not* bold in a heading is more important to me than knowing something is bold, since bold tends to be a default attribute for a heading.

> 3. We leave it as you have it here, but do the additive thing to set the
> default value to Default in the soffice settings.
...
> (My rationale for 2 and 3 is this: If I press Orca+F I only care about the
> paragraph style if it's a heading or something other than Default.)

Heh - I guess we disagree.  Knowing something is the "Default" style is just as important to me as knowing it is the "Heading 1" or "Text body" style.

> In addition, if we've determined that paragraph-style should be presented, I
> would think I'd want to see it in braille as well -- as long as it's not a
> style of "Default".

I'm not quite sure what the right thing to do is with respect to raising dots on the display for these things.

> +        #
> +        self.additionalSpokenTextAttributes = \
> +          "; left-margin:0mm; right-margin:0mm;"
> 
> -        self.enabledBrailledTextAttributes = \
> -          "size:; family-name:; weight:400; indent:0mm; left-margin:0mm; " \
> -          "right-margin:0mm; underline:none; strikethrough:none; " \
> -          "justification:left; style:normal;"
> -        self.enabledSpokenTextAttributes = \
> -          "size:; family-name:; weight:400; indent:0mm; left-margin:0mm; " \
> -          "right-margin:0mm; underline:none; strikethrough:none; " \
> -          "justification:left; style:normal;"
> 
> Removing these has some additional side effects it seems. In particular:
> 
> 1. Elsewhere, the default value of indent is 0; in OOo it's Omm. 

Aha - I was led astray by the comment that said the only difference was with respect to the margins.  Silly me.  ;-)

I'm wondering if we might want to to make the values be regular expressions. A regular expression could match 0 and 0mm for indent for example.  Regular expressions might also allow sophisticated users to express more than one thing that should not be brailled or spoken, and they would also allow for paragraph styles such as "Default", "Text body", and "My custom default text body which is what I have made as the default and I do not want it presented" to not be brailled or spoken in the event they don't find knowing these things as important as I do.  :-)

> 2. Elsewhere the default value of strikethrough is apparently false; it's None
> in OOo. 

Regular expressions might help here, too.

(In reply to comment #10)
> Tried the whereAmI patch.
..
> SPEECH OUTPUT: ' ;  paragraph style Default weight 400 This  ;  paragraph 

Heh - this looks like it might have been a general bug that was exposed by this patch.  If a weight of 400 ever showed up in text.getAttributes(textOffset), I believe this would have been exposed.  The Where Am I code probably needs something along the lines of default.py:outputCharAttributes (or maybe it can get by with calling it, which would be better if it could be done).

I will try to work up a patch that adds regular expressions and fixes the Where Am I bug with weight == 400.

Not sure how I did all those changes. Oops.

Created attachment 132291 [details] [review]
Combined patch

This patch fixes the Where Am I problem with weight and it also keeps the speaking of the paragraph style down to just once per Where Am I regardless of how many times the attributes change.  

It also accounts for the indent and strikethrough value differences by doing a string replace instead of doing the regular expression idea, and it keeps the braille stuff kind of where it was.  I started doing the regular expression idea, but it got more complex than I had intended due to us doing things like checking for weight and bold and such.  So, I just punted and went for the easy option of a string replace.

Patch committed to trunk and gnome-2-26.  One small change to the patch was made, which was to do this

+        self._lastAttributeString = ""

instead of this:

+        self._lastAttributeString = None

to make it more consistent with how _lastAttributeString is initialized in the module (thanks Joanie!).