This is another OOo bug. We are always getting "indent:0;" back
irrespective of the actual text indentation.

I've opened OOo issue #72262 against this
http://www.openoffice.org/issues/show_bug.cgi?id=72262
and adjusted the summary of this bug accordingly.

From a comment that Oliver made to Will today, it seems that the
text attribute that's getting adjust here is "left-margin:".

I just did the instructions I wrote for OOo issue #72161, namely:

  Startup Writer and create a small document with:

  The quick
  brown fox
  jumps over
  the lazy dog.

  Position the text caret at the beginning of the "brown fox" line
  and use the "Increase Indent" icon from the toolbar line to add
  an indentation to this line.

I added a couple of lines of debug to readCharAttributes() in default.py
and got the following (Insert-f for the first two lines of the document):


readCharAttributes: default text attributes: size:12; left-margin:0; pixels-below-lines:0; pixels-above-lines:0; variant:normal; invisible:false; right-margin:0; strikethrough:false; underline:none; indent:0; style:normal; weight:400; justification:left
readCharAttributes: char attrs:  ('', 0, 10)

readCharAttributes: default text attributes: size:12; left-margin:47; pixels-below-lines:0; pixels-above-lines:0; variant:normal; invisible:false; right-margin:0; strikethrough:false; underline:none; indent:0; style:normal; weight:400; justification:left
readCharAttributes: char attrs:  ('', 0, 10)

We can see that "left-margin:" has changed from 0 to 47.
Knowing this we can probably script around this. The default value
for enabledTextAttributes in settings.py is:

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

I suggest we just adjust this in the StarOffice.py script to:

enabledTextAttributes = "size:; family-name:; weight:400; left-margin:0; underline:none; strikethrough:false; justification:left; style:normal;"

Does that seem reasonable?

Created attachment 79396 [details] [review]
Patch to implement the suggested workaround.

This seems to work nicely (assuming everybody is okay 
with it saying "left margin 47").

Please let me know if you are okay with this. If so, I'll check it in.

> This seems to work nicely (assuming everybody is okay 
> with it saying "left margin 47").

47 whats?

I think this looks fine.  What would the impact be of leaving "indent" in there and just adding "left-margin"?  In addition, what does the number 47 mean?  It is millimeters, pixels, a percentage?

I've no idea what 47 means. I'd guess pixels. It's not millimetres or 
a precentage. Yes, we could leave "indent" in there too.

For now, I'll check in the existing patch (plus "indent") and leave the
bug open until we can find out from Oliver what the 47 means. Then we
can add some most custom scripting to StarOffice.py to speak the units
as well.

It'll at least be an improvement over what we currently have. Thanks.

Change checked in (plus "indent"). Email sent to Oliver. I'll
update with what he says. On reflection, I'm going to guess 
that they are points.

Depending on what Oliver says it would be really nice to take this value and calculate some meaningful unit of measure from it.   

Just tried the patch.  Very cool!

Since this is still open:  How about adding right-margin (which also defaults to 0)?  I just made that change and then created a blockquote in Writer by indenting from the left and dragging the ruler to move the end of the line to 6 inches.  With the right margin addition to StarOffice.py, I can now distinguish blockquotes from left-side indents and also tell if my blockquote is balanced (i.e. has the same amount of indentation on both sides).

Good idea. Done (and checked into SVN trunk). Thanks!

w.r.t. comment #8, if you take the reported value and divide it by 95, you'll get the margin in inches give or take a couple hundredths of an inch.  I verified this for a variety of fonts, styles, font sizes, zoom levels, etc.

On another note, it just occurred to me:  We still aren't getting indentation if the indentation was accomplished via tabbing.  Maybe that's what indent is for? 

Sorry to be spammy, but I just found something else:  If you have a negative indent (e.g. -47) and have punctuation set to some or none, you lose the minus sign in speech.  I suppose this is a broader issue so let me know if you'd like me to file it as such.

Since I'm already commenting....  Rich:  Your guess about the mystery number being points seems right: 95-point text is 1 inch tall (and an indent of 95 whatevers means you're indented 1 inch).

> I suppose this is a broader issue so let me know if you'd like
> me to file it as such.

Yes. Please file a new bug on that one. Thanks.

OpenOffice.org bridges to ATK, not to at-spi. The ATK docs (http://developer.gnome.org/doc/API/2.0/atk/AtkText.html#AtkTextAttribute) say:

ATK_TEXT_ATTR_LEFT_MARGIN	 The pixel width of the left margin

OOo uses 100th of mm internally, so if that would suit you better, we could expose this as additional attribute in OOo 2.2.

Thanks Oliver. So Mike/Joanie et al, what would you like spoken
here?  I don't think we can convert to mm's as that'll confuse
most American's. Maybe leave as "pixels"...

Please let me know and I'll adjust the script accordingly.

Could we possibly calculate perhaps 10ths of an inch?  Perhaps we could hear indented 1.2 inches or something like that.  

The problem here is that most of the rest of the world is using the
metric system. (It's only countries like America that's got it's head 
firmly up its butt and still using a useless archaic measuring system. 
1/2 :-) )

Our GNOME/Gtk+ localization facilities don't make it easy to swap from one 
system to another.

Will suggested (also half jokingly) that it would be nice to try
to determine what units the OOo ruler is using and use that, but I
don't believe that that is possible.

For now I'm just going to get it to speak "pixels". I don't
currently know how hard that is, so I'm not sure if this will make
it for the Orca 2.17.5 tarball on Sunday.

Created attachment 79464 [details] [review]
Patch to add in speaking "pixels".

Simple patch as it turns out. As it's part of the ATK spec,
I've adjusted the default.py code to handle this (as opposed to
making it specific for OOo).

Stupid question time (must be because I'm American ;-) )

I realize what Oliver quoted, but aren't "pixels" the little dots that make up what is being displayed?  If so, given a document with indentation, wouldn't one expect different results at different resolutions? My left margin is consistently 47 regardless of resolution.  Aren't these points rather than pixels??  Sorry to be dense....

You are absolutely right, which suggests (to me at least), that 
perhaps the ATK docs at:
http://developer.gnome.org/doc/API/2.0/atk/AtkText.html#AtkTextAttribute
which currently say:

ATK_TEXT_ATTR_LEFT_MARGIN        The pixel width of the left margin

should be updated to use a better unit of measurement.

Even though I am clearly not an toolit expert, I tend to disagree: the 100th mm/point units are valid for printing, but not fo displaying on the screen. This is mostly because the application would not only need the screen resolution, but also the screen width the calculate the correct number of pixels, which I believe is not possible.

If you change the resultion of your screen, your document still uses the same static dpi mapping, which you can get from the fact that you see more of your document when you increase the resolution. If the mapping were dynamic, the display size of the document would have to stay the same.

I have to take back what I wrote above: screen resolution can be queried and should be honored by OOo. Sorry for the confusion (and the delay).

However, meanwhile I has the followin conversation with Bill Hanemann on this:

Oliver Braun wrote:
<quote>
many of the ATK text attributes are specified to use pixels as unit of measure, which - at least in theory - depends on the resolution and size of the monitor one uses.

I find this rather unsatisfying for several reasons:
- CSS do not have this limitation, they allow in, cm , mm, pt, pc for fixed values, sometimes percent and even "auto"
- OOo uses "100th of mm", "percent" and "auto" internally
- Orca users have expressed they would prefer a unit of measure more meaningful to them to be spoken.

So I wonder what the point in forcing app & at-tool to calculate values forth and back is. Specifically I have the problem of having to "guess" the value for "auto" on superscript.

[..]

What do you think is the best way to proceed ?
</quote>

Bill Hanemann wrote:
<quote>
I think it may be best to allow ATK's value to include an (optional) units string, i.e

10mm

or 25pt, as long as this matches the equivalent strings in the CSS spec.
</quote>


So, beside that there seems to be a bug in OOo or Xorg, which results in incorrect margin values at least for some screen reslutions, we might head for using a different unit of measure (OOo uses 100th of mm internally) and change the ATK specification accordingly. What do you think ?

I may be coming at this from the wrong angle, so please correct me if I'm misunderstanding the problem.

> I think it may be best to allow ATK's value to include an (optional) units
> string, i.e
> 
> 10mm
> 
> or 25pt, as long as this matches the equivalent strings in the CSS spec.

Having an optional units string appended to the value seems reasonable to me.

> So, beside that there seems to be a bug in OOo or Xorg, which results in
> incorrect margin values at least for some screen reslutions, we might head for
> using a different unit of measure (OOo uses 100th of mm internally) and change
> the ATK specification accordingly. What do you think ?

Without the optional units string, it seems as though we need to have an implicit unit for those things without the optional units string.  If ATK/AT-SPI currently implies something (is it pixels?), then that is probably the way to go rather than modifying the spec.

(In reply to comment #23)
> Without the optional units string, it seems as though we need to have an
> implicit unit for those things without the optional units string.  If
> ATK/AT-SPI currently implies something (is it pixels?), then that is probably
> the way to go rather than modifying the spec.
>

ATK/AT-SPI are somewhat inconsistent at this point: AT-SPI just specifies "The text attributes correspond to CSS attributes where possible", which would mean one unit of in, cm , mm, pt, pc or px, while ATK limits this choice to px.

The mapping from ATK to AT-SPI has some more problems in this case:

1) the CSS string is "margin-left", not "left-margin" (http://www.w3.org/TR/CSS21/box.html#margin-properties)

2) ATK seems not to insert the unit of measure in the string

Maybe OOo should ignore the ATKTEXT_ATTR_*_MARGIN constants and directly return the CSS values ?

> Maybe OOo should ignore the ATKTEXT_ATTR_*_MARGIN constants and directly return
> the CSS values ?

If there's a standard in place (e.g., CSS), then it seems to me to make sense to use it.  Li, can you comment on this from the AT-SPI/ATK perspective? 

Appending unit to the value is a good choice to me. And I prefer the default value is pixels. Is there any code need to be changed except document?

I mean what I need to do is to change the definition of ATKTEXT_ATTR_* . And OOo will append unit to value and Orca will read it, right?

Sounds reasonable to me. 

The only optional addition to this would be to have the at-spi/atkbridge add the unit of measure symbol (px) for applications not specifying it, but this would probably break some ATs for all applications, not "just" OOo.

(In reply to comment #28)
> Sounds reasonable to me. 
> 
> The only optional addition to this would be to have the at-spi/atkbridge add
> the unit of measure symbol (px) for applications not specifying it, but this
> would probably break some ATs for all applications, not "just" OOo.

I think this sounds OK, too.  To help prevent some breakage, perhaps 'px' need not be added, since that is the current default unit?

Rich, Joanie - what do you think of this?

I see Bugzilla is back again now.

That's fine with me. We can easily adjust the StarOffice.py script
to add in speaking/brailling "pixels" if it's the default. I don't
think we want to try to get too clever and handle "localizing" into
inches or cms. It'll just complicate the code.

(In reply to comment #29)
> I think this sounds OK, too.  To help prevent some breakage, perhaps 'px' need
> not be added, since that is the current default unit?

I'm not sure I'm following. Is px merely the default unit or is it the only unit?  

If the only unit is px, I don't see the need to add it on if doing so might break things.  Like Rich said, he can do that in StarOffice.py. If other units are possible, would we still add on the unit?  If not, because of possible breakage, how can we provide access to the unit?

I think px is just the default unit. We can use other unit like mm or pt. If applications want to use unit which is not px, they should append the unit to the value. If there is not unit in the value, the unit will be px.

(In reply to comment #32)
> I think px is just the default unit. We can use other unit like mm or pt. If
> applications want to use unit which is not px, they should append the unit to
> the value. If there is not unit in the value, the unit will be px.

Is the unit text a localized form or a computer form?  That is, will we need to translate it as appropriate?

What is CSS using? Localized form or computer form?

The CSS is intended to be interpreted by a browser, thus it is using the computer form (see http://www.w3.org/TR/CSS21/syndata.html#value-def-length).

So the atk spec should be changed to allow optional unit strings (in computer form, as defined by CSS). The at-spi documentation doesn't specify any attributes, otherwise I would have proposed to document the default of 'px' there. 

Regarding localization, I think the application should honor the users preferences and pass the same unit (in computer form) e.g. the rulers are using.

OOo currently supports centimeter, millimeter, inch, pica and point, which map to the CSS values "cm", "mm", "in", "pc" und "pt". The mapping between computer form back to localized form would have to be done by orca (based on the language it is configured to read).

Please note that in CSS there often is also a value "auto", which might be used by OOo as well.

Removing target milestone from [blocked] bugs.  We have little control over them, so we're better off letting priority and severity be our guide for poking the related components.

In looking at the current code in trunk, I see support for "pixels" output at line 3314 in default.py:outputCharAttributes.  Does that address this bug?  In addition, I think we probably should consider ngettext for the current line at 3314.  Rich, can you look into doing this?

Created attachment 96651 [details] [review]
Reworked patch  to add ngettext support as requested in comment #37 and use correct line numbers for previous diff.

Oh, and yes, we are still blocked.

Actually, I take it back. We are not blocked. That's the OOo metabug.
Can others please test this when they have a spare moment. Thanks!

I get: SPEECH OUTPUT: 'left-margin 12.51mm pixels'

I'm not sure what you tested to get that, so here's that I just tried:

Steps to reproduce:

1/ Start Orca from SVN trunk. Make sure that you are speaking 
   indentation. I.e:
     orca.settings.enableSpeechIndentation = True

2/ Start oowriter

3/ Enter (at one level of indent) "The quick brown fox." followed by Return.

4/ Press up arrow to get back to that first line.

For me, Orca speaks:

SPEECH OUTPUT: 'The quick brown fox.'

This seems wrong. I would have expected it to announce the indentation.
I'm investigating that.

If I then do Insert-f, I get:

SPEECH OUTPUT: 'size 12'
SPEECH OUTPUT: 'family-name Times'
SPEECH OUTPUT: 'left-margin 12.51mm'

So...

1/ What were you testing?
2/ What do you get for the above test?

Thanks.

Okay, so it's not speaking the indentation because
speakTextIndentation() in default.py only speaks
indentation if it's tabs or spaces. That's not what
we've got here.

1.  Essentially the same test.
2.  Same thing as before.

readCharAttributes: default text attributes: line-height:100%; paragraph-style:Default; justification:left; pixels-below-lines:0mm; pixels-above-lines:0mm; indent:0mm; right-margin:0mm; left-margin:12.51mm; vertical-align:baseline; writing-mode:lr-tb; text-shadow:none; text-rotation:0; text-decoration:none; font-effect:none; stretch:normal; direction:ltr; language:en-us; scale:1; style:normal; variant:normal; family-name:Times; weight:400; size:12; strikethrough:none; underline:none; invisible:false; fg-color:0,0,0; bg-color:255,255,255
SPEECH OUTPUT: 'size 12'
SPEECH OUTPUT: 'family-name Times'
SPEECH OUTPUT: 'left-margin 12.51mm pixels'

Well whoop-de-doo! 

Here's my readCharAttributes debug line:

readCharAttributes: default text attributes: line-height:100%; paragraph-style:Default; justification:left; pixels-below-lines:0mm; pixels-above-lines:0mm; indent:0mm; right-margin:0mm; left-margin:12.51mm; vertical-align:baseline; writing-mode:lr-tb; text-shadow:none; text-rotation:0; text-decoration:none; font-effect:none; stretch:normal; direction:ltr; language:en-us; scale:1; style:normal; variant:normal; family-name:Times; weight:400; size:12; strikethrough:none; underline:none; invisible:false; fg-color:0,0,0; bg-color:255,255,255

I have no idea why you are getting something different than me.

Created attachment 96655 [details] [review]
Re-revised patch.

Ack! I transcribed it wrong. Try this one.

Thanks to Joanie for pointing out my mistake (quietly on IRC).

Created attachment 96656 [details] [review]
Re-re-revised patch.

Sigh. Must...get...sleep...

Created attachment 96657 [details] [review]
Fourth time's a charm! (I hope).

Joanie and I worked out what I was doing wrong. It seems
that that previous first patch had been rewritten at some
point and already incorporated it a different way. 

The new patch now just does the ngettext change.

If I've got it wrong this time, I'm going to leave it until
another day!

Works beautifully! :-)

Thanks Joanie. Patch committed to SVN trunk. Setting bug to "[pending]" state.

Closing as FIXED.