After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 681591 - Masked string for password fields is not exposed to accessibility
Masked string for password fields is not exposed to accessibility
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Accessibility
3.5.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2012-08-10 09:41 UTC by Mario Sánchez Prada
Modified: 2012-08-14 23:48 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch proposal (5.62 KB, patch)
2012-08-10 10:18 UTC, Mario Sánchez Prada
none Details | Review
Patch proposal v2 (16.24 KB, patch)
2012-08-10 14:10 UTC, Mario Sánchez Prada
none Details | Review

Description Mario Sánchez Prada 2012-08-10 09:41:17 UTC
STEPS TO REPRODUCE:

 1. Run an application showing a GtkEntry with 'visibility' property set to FALSE, meant
     to be used to capture a password from the user (e.g. gtk3-demo, in "Entry" > "Entry buffer")
 2. Type some text in the password field, which will display that text masked (e.g. "******")
 3. Try to access the text in that field through Assistive Technologies (e.g. Orca, Accerciser), by
     using one of the following functions from the AtkInterface:
        - atk_text_get_text()
        - atk_text_get_text_before_offset()
        - atk_text_get_text_at_offset()
        - atk_text_get_text_after_offset()

EXPECTED OUTCOME:

Providing the password field is not empty, you should be getting the actual "masked" string being displayed in the field when calling any of those functions with the right parameters.

For instance, if you typed "12345" in the password field, atk_text_get_text (accessible, 0, -1) should return something like "*****" (where 'accessible' is the instance of GtkEntryAccessible for that instance of GtkEntry)

ACTUAL OUTCOME:

Whenever you call any of those functions over that instance of GtkEntryAccessible, you always get an empty string, as it's hardcoded to be like that in the code:

   static gchar *
   gtk_entry_accessible_get_text (AtkText *atk_text,
                                                  gint start_pos,
                                                  gint end_pos)
   {
      [...]

      /* FIXME: is this acceptable ? */
     if (!gtk_entry_get_visibility (GTK_ENTRY (widget)))
       return g_strdup ("");

      text = gtk_entry_get_text (GTK_ENTRY (widget));
      [...]
   }

Btw, this problem is also causing that no proper "text-change"/"text-insert"/text-remove" signal is being emitted whenever the user inserts/removes new characters in those password fields, making impossible for users of ATs to realize that something happened in there.

I know just exposing the number of characters for a password might be considered a security hole, but from the point of view of ATs like Orca I think it's very interesting to be able to actually deal with these kind of input fields as if they were any other of entries, so users of ATs can interact with those kind of fields in a proper way. For what is worth, this is what Firefox does already for <input type="password"> fields and what we are planning to do in WebKitGTK+ (patch not landed yet)

Adding Piñeiro and Joanmarie Diggs to CC since I'm sure they will be interested in tracking this bug down (and maybe in adding some extra info here)

I'll be attaching a patch soon
Comment 1 Mario Sánchez Prada 2012-08-10 10:18:10 UTC
Created attachment 220860 [details] [review]
Patch proposal

Attaching a new patch proposal for fixing this issue.

I previously added Benjamin to CC thinking of him as a possible reviewer for this patch, but feel free to point me to someone else if that's the case.

Thanks!
Comment 2 Mario Sánchez Prada 2012-08-10 14:10:29 UTC
Created attachment 220890 [details] [review]
Patch proposal v2

After discussing a bit with Benjamin over IRC, we agreed that there should be a better way to fix this, by making the gtk_entry_get_display_text() private function in gtkentry.c available to gtkentryaccessible.c via gtkentryprivate.h (and dealing with returning new memory instead of a const gchar*).

So, this new patch is for that. I also updated the tests for entries (entries.[ui|txt]) to include a new entry field with 'visibility' set to FALSE, so we can check this does not break by mistake in the future.

Hence, asking for review before pushing. Thanks
Comment 3 Mario Sánchez Prada 2012-08-10 16:12:59 UTC
Benjamin confirmed via IRC that I could commit this patch, so did I (although I did it in 3 separate patches, as per his recommendation)

Thus, this problem has been fixed in the development version. The fix will be available in the next major software release.
Comment 4 Colin Walters 2012-08-14 23:44:26 UTC
There seems to have been an unintentional hunk in this patch:

-void      _gtk_entry_get_borders            (GtkEntry  *entry,
-                                             GtkBorder *borders);
+void     _gtk_entry_get_border             (GtkEntry  *entry,
+                                            GtkBorder *borders);


Why?
Comment 5 Colin Walters 2012-08-14 23:48:50 UTC
Nevermind, I just fixed it.