Bug 105778 – REGRESSION(r138394): platform/mac/accessibility/file-upload-button-subrole.html is failing

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[Ryosuke Niwa]

http://trac.webkit.org/changeset/138394
@@ -621,5 +639,5 @@

 /* accessibility role description for a file upload button */
-"file upload" = "file upload button";
+"file upload" = "file upload";

caused platform/mac/accessibility/file-upload-button-subrole.html to start failing.

http://build.webkit.org/results/Apple%20Lion%20Release%20WK1%20(Tests)/r138484%20(7648)/platform/mac/accessibility/file-upload-button-subrole-pretty-diff.html

[Ryosuke Niwa]

Mountain Lion results: http://build.webkit.org/results/Apple%20MountainLion%20Release%20WK1%20(Tests)/r138484%20(5162)/results.html

[Ryosuke Niwa]

Added a test expectation in http://trac.webkit.org/changeset/138486.

[Alexey Proskuryakov]

This part of the patch should be reverted.

[Brady Eidson]

From my comment over in 105592, https://bugs.webkit.org/show_bug.cgi?id=105592#c9

"That happened simply because I ran update-localizable-strings (or whatever it is actually called) which apparently nobody had done in awhile.
If code changed that caused the generated string to be "file upload button" and that code was correct, then we should rebaseline.
I have no idea how to explore that."

Alexey, you say that part of the change should be reverted.  How do you determine this?

I ask because if someone reverted it then checked that in...  then someone later came along and ran update-webkit-localizable-strings again, the same change would be generated.

It seems if update-webkit-localizable-strings will always regenerate that change, yet that change is wrong, then either update-webkit-localizable-strings is broken or something in the code is broken.

And whichever of those is broken should be fixed.

[Alexey Proskuryakov]

> Alexey, you say that part of the change should be reverted.  How do you determine this?

Such a change would require review, not a rubber-stamp. Accessibility behavior changes should be discussed on their own merits, not be kept in the tree after an accidental commit by default.

"file upload button" certainly sounds like a more appropriate plain English description for the button, if I were to guess about the rationale for existing behavior.

I think that update-webkit-localizable-strings just always makes English key and value match, which wasn't the case before. You can use svn blame to determine when they were made different. Perhaps someone fixed the text in Localizable.strings without realizing that don't treat the file as authoritative.

[Ryosuke Niwa]

What are we going to do about this?

[chris fleizach]

we should change the strings file to use

"file upload button" = "file upload button";

[Ryosuke Niwa]

Created attachment 181389 [details]
Fixes the bug

[WebKit Review Bot]

Attachment 181389 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1
LayoutTests/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Ryosuke Niwa]

Comment on attachment 181389 [details]
Fixes the bug

View in context: https://bugs.webkit.org/attachment.cgi?id=181389&action=review

>> LayoutTests/ChangeLog:8
>> +	Removed the old test expectations.
> 
> Line contains tab character.  [whitespace/tab] [5]

Fixes this. Thanks for the review, Alexey.

[Radar WebKit Bug Importer]

<rdar://problem/12960393>

[Ryosuke Niwa]

Committed r138860: <http://trac.webkit.org/changeset/138860>