Escapes backslash, colon, comma, equals and semicolon in IAccessible2 string attributes.

Escapes backslash, colon, comma, equals and semicolon in IAccessible2 string attributes.

This is a requirement of the IAccessible2 object attributes specification and is affecting NVDA.
BUG=540974
R=aboxhall@chromium.org, dmazzoni@chromium.org, dtseng@chromium.org

Committed: https://crrev.com/bd081903b5e478d5d6b32daca4549d84a5203058
Cr-Commit-Position: refs/heads/master@{#353671}

[dmazzoni, 2015-10-12 17:15:15]

https://codereview.chromium.org/1397183002/diff/2/content/browser/accessibili...
File content/browser/accessibility/browser_accessibility_win.cc (right):

https://codereview.chromium.org/1397183002/diff/2/content/browser/accessibili...
content/browser/accessibility/browser_accessibility_win.cc:3258:
win_attributes_->ia2_attributes.push_back(
There are other places where we're adding strings to the ia2_attributes vector
too, like this one - you should make sure those are escaped too.

https://codereview.chromium.org/1397183002/diff/2/content/browser/accessibili...
content/browser/accessibility/browser_accessibility_win.cc:3642:
base::ReplaceChars(input, L":", L"\\:", output);
This won't work correctly if input and output are different. In the calling
function, input and output are the same and it happens to work in that case.

[chromium-reviews, 2015-10-12 19:38:38]

All fixed.
Added more unit testing to prevent errors like the one I made.

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[dmazzoni, 2015-10-12 20:53:50]

lgtm

https://codereview.chromium.org/1397183002/diff/30001/content/browser/accessi...
File content/browser/accessibility/browser_accessibility_win.h (right):

https://codereview.chromium.org/1397183002/diff/30001/content/browser/accessi...
content/browser/accessibility/browser_accessibility_win.h:754: // Escapes the
following characters with backslash: backslash, colon, comma,
Nit: I'd prefer if you didn't list which characters are escaped here in the
header file. If someday in the future somebody changed it, it'd be easy to
forget to change the header file comment. Have this comment just explain that it
escapes the characters necessary according to the IAccessible2 spec. The cc file
can document what characters those are.

[chromium-reviews, 2015-10-13 02:20:37]

Header file updated.
Also, used FRIEND_TEST_ALL_PREFIXES(BrowserAccessibilityTest, 
TestSanitizeStringAttributeForIA2); to expose private method to unit test.
Submitting.

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[nektarios, 2015-10-13 02:20:51]

The CQ bit was checked by nektar@chromium.org

[nektarios, 2015-10-13 02:20:51]

The patchset sent to the CQ was uploaded after l-g-t-m from
dmazzoni@chromium.org
Link to the patchset: https://codereview.chromium.org/1397183002/#ps50001
(title: "Made private method testable.")

[commit-bot: I haz the power, 2015-10-13 02:21:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1397183002/50001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1397183002/50001

[commit-bot: I haz the power, 2015-10-13 02:22:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-13 02:22:45]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[nektarios, 2015-10-13 03:08:53]

The CQ bit was checked by nektar@chromium.org

[nektarios, 2015-10-13 03:08:54]

The patchset sent to the CQ was uploaded after l-g-t-m from
dmazzoni@chromium.org
Link to the patchset: https://codereview.chromium.org/1397183002/#ps70001
(title: "Rebased with master.")

[commit-bot: I haz the power, 2015-10-13 03:09:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1397183002/70001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1397183002/70001

[commit-bot: I haz the power, 2015-10-13 03:54:11]

Committed patchset #5 (id:70001)

[commit-bot: I haz the power, 2015-10-13 03:55:04]

Patchset 5 (id:??) landed as
https://crrev.com/bd081903b5e478d5d6b32daca4549d84a5203058
Cr-Commit-Position: refs/heads/master@{#353671}