Bug 53033 - [GTK] Space characters in source document interfere with reported caret offset
Summary: [GTK] Space characters in source document interfere with reported caret offset
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks: 25531
  Show dependency treegraph
 
Reported: 2011-01-24 11:41 PST by Joanmarie Diggs
Modified: 2011-01-27 11:38 PST (History)
2 users (show)

See Also:


Attachments
test case (612 bytes, text/html)
2011-01-24 11:41 PST, Joanmarie Diggs
no flags Details
Patch proposal + unit tests (8.40 KB, patch)
2011-01-27 10:56 PST, Mario Sanchez Prada
mrobinson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joanmarie Diggs 2011-01-24 11:41:49 PST
Created attachment 79949 [details]
test case

There are three paragraphs in the attached test case. Each paragraph's (displayed) text is the same length. Aside from the different word order in paragraph three, what distinguishes these paragraphs is whitespace in the source document:

* p1 - no extraneous whitespace
* p2 - one word per line (i.e. extraneous newline chars)
* p3 - one word per line plus crazy use of space chars

p1 and p2 seem to work as expected for all aspects of AtkText. p3 has problems with the reported caret offset (both during caret-moved events and when the caretOffset is requested).

From Accerciser:

In [1]: p1 = acc.queryText()
In [2]: p1.getText(0, -1)
Out[2]: 'Is this paragraph borked?'
In [3]: p2 = acc.queryText()
In [4]: p2.getText(0, -1)
Out[4]: 'Is this paragraph borked?'
In [5]: p3 = acc.queryText()
In [6]: p3.getText(0, -1)
Out[6]: 'This paragraph is borked!'
In [7]: p1.characterCount
Out[7]: 25
In [8]: p2.characterCount
Out[8]: 25
In [9]: p3.characterCount
Out[9]: 25

^^^ Proof that most things are ok :-) ^^^

When moving by word, starting from the top of the file in Epiphany:

object:text-caret-moved(2, 0, None)
	source: [paragraph | ]
	application: [application | epiphany-browser]
object:text-caret-moved(7, 0, None)
	source: [paragraph | ]
	application: [application | epiphany-browser]
object:text-caret-moved(17, 0, None)
	source: [paragraph | ]
	application: [application | epiphany-browser]
object:text-caret-moved(24, 0, None)
	source: [paragraph | ]
	application: [application | epiphany-browser]
object:text-caret-moved(2, 0, None)
	source: [paragraph | ]
	application: [application | epiphany-browser]
object:text-caret-moved(7, 0, None)
	source: [paragraph | ]
	application: [application | epiphany-browser]
object:text-caret-moved(17, 0, None)
	source: [paragraph | ]
	application: [application | epiphany-browser]
object:text-caret-moved(24, 0, None)
	source: [paragraph | ]
	application: [application | epiphany-browser]
object:text-caret-moved(4, 0, None)
	source: [paragraph | ]
	application: [application | epiphany-browser]

^^^ This is all fine too. :-) ^^^

But here's where the crazy whitespace begins in the source document:

object:text-caret-moved(40, 0, None)
	source: [paragraph | ]
	application: [application | epiphany-browser]
object:text-caret-moved(97, 0, None)
	source: [paragraph | ]
	application: [application | epiphany-browser]
object:text-caret-moved(468, 0, None) 
	source: [paragraph | ]
	application: [application | epiphany-browser]

As a reminder:
In [9]: p3.characterCount
Out[9]: 25

;-) ;-)

Also at this point:
In [10]: p3.caretOffset
Out[10]: 468

In terms of WebKit Bugzilla, I cannot justify calling this a P1. BUT, in terms of other priorities, including Yelp 3 accessibility... If possible, please drop everything else and consider this a P1. The impact of this bug is Orca's going to get bogus caret-moved events and speak the wrong words any time there is extra space characters present. This will also interfere with Orca's ability to correctly report text selection. And we need sufficient time for the fixed WebKitGtk to get picked up downstream before GNOME 3.

Sorry I just now noticed this and thanks in advance for prioritizing it!!
Comment 1 Mario Sanchez Prada 2011-01-27 10:56:42 PST
Created attachment 80348 [details]
Patch proposal + unit tests

Attaching a patch proposal and the new unit test needed.
Comment 2 Martin Robinson 2011-01-27 11:01:18 PST
Comment on attachment 80348 [details]
Patch proposal + unit tests

Looks sane!
Comment 3 Mario Sanchez Prada 2011-01-27 11:38:23 PST
Committed r76822: <http://trac.webkit.org/changeset/76822>