862555 - Use correct flags for the Edit Bookmark URL field

Status: RESOLVED WORKSFORME

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[Terry]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_7_5) AppleWebKit/537.31 (KHTML, like Gecko) Chrome/26.0.1410.65 Safari/537.31

Steps to reproduce:

Originally bug 823639, which I worked on for a school project. A fix was found and implemented with success in the XML bookmark_edit file. Look to bug 823639 for more details.


Actual results:

The fix worked on our devices and was confirmed by others, but the bug resurfaced sometime in the two weeks before our demonstration day. Upon some quick testing, I realized the XML fix no longer worked for my Nexus 7 running Android 4.2.2 and SwiftKey Tablet (most current version at the time).


Expected results:

The bookmark editing should not allow keyboards to offer suggestions when editing URLs. The bug resurfaced either do to some other factors in the build or the SwiftKey update.

Comment 1

[Terry]

I will be testing with just the Java fix to see if that works on a couple devices as per a suggestion while chatting on IRC. If that works, I will re-patch with only the Java change.

In the meantime, if someone could double-check this situation and see if the problem is still persisting on devices other than Google Nexus, that would be helpful.

Please refer to the related bug mentioned above before asking questions. Thank you.

Comment 2

[Aaron Train [:aaronmt]]

Renaming to emphasize that this is an issue on some devices.

Can these devices be narrowed down; as in, are these problematic on devices running TouchWiz? SenseUI? Blur? Specific Android versions? Etc

Comment 3

[Terry]

I can only confirm this on my own device so far, and that would be a Google Nexus 7 running Android 4.2.2 and a recent trunk version of Firefox.

As far as I know, this bug was occurring on all devices we came across in the previous version of this bug, but I am assuming that by renaming this bug you are implying that you know of devices where this bug is not resurfaced.

Please list what devices that do not show this bug, so that I may check back with those once I submit a Java-only fix.

P.S. I'm moving my build and development to a Desktop, but I ran into some issues so it may take a while before the patch submit.

Thanks

Comment 4

[Terry]

Attachment: Added the Java file fix and removed the XML fix that was previously accepted.

The Java-only fix works on my Google Nexus 7 running Android 4.2.2 and a trunk version of Firefox downloaded earlier yesterday.

This was tested while using SwiftKey and SwiftKey Tablet (both version 4.0.1.128) and was successful in both cases.

Comment 5

[Terry]

I spent a good portion of this weekend working out my problems with Linux, but the good news is I am successfully developing on my desktop now. Feels so much better than doing it on a laptop.

Anyway, even better news: I have a new patch with only a Java fix implemented, and it works perfectly on my Nexus.

Can someone please flag an appropriate reviewer for this or suggest someone for me to flag?

Now, I am just awaiting feedback regarding the new fix on other devices.

Thanks.

Comment 6

[Brian Nicholson (:bnicholson)]

We use "textNoSuggestions" for AwesomeBar's EditText, but we also include the "textUri" inputType; see http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/layout/awesomebar_search.xml#35. Since edit_bookmark_location is also a URI-based EditText, I imagine that we also want to use textUri here. Have you tried the combined "textUri|textNoSuggestions" flags in XML?

CC'ing jchen in case he has any input (no pun intended).

Comment 7

[Terry]

Interesting! OK, so I did a cursory search for the definitions of what each inputType means, and from what I can tell this means that the equivalent of the " TYPE_TEXT_FLAG_NO_SUGGESTIONS" is already being implemented for the AwesomeBar, yet suggestions still show up on my device.

FYI:
http://developer.android.com/reference/android/R.styleable.html#TextView_inputType

I tried the fix you recommended, and it seems that the keyboard did indeed conform to a more URI-friendly mode and provide a ".com" key (pretty sure that wasn't there before) and it also fixes the problem of the IME auto-inserting a space for every period. However, text suggestions still show up. Is this what we want? I thought the primary issue was having any suggestions whatsoever since URLs are often not dictionary-based words.

Let me know what you guys think and I will patch again if appropriate.

Comment 8

[Terry]

(In reply to Brian Nicholson (:bnicholson) from comment #6)
> We use "textNoSuggestions" for AwesomeBar's EditText, but we also include
> the "textUri" inputType; see

I think the AwesomeBar's EditText affects the main search bar (is that correct?).

In which case, it makes perfect sense to keep the main search bar and the editing of bookmark URLs parallel in terms of implementation. The issue is that I am just now realizing that the main search bar also offers suggestions, and I don't think that is what we want given the original bug statement.

If this is NOT what we want, then I suppose the fact that the main search provides suggestions is a related bug in and of itself.

Comment 9

[Brian Nicholson (:bnicholson)]

I agree that we should keep the keyboard behavior for the AwesomeBar URL bar and the bookmark URL the same. I'm not sure suggestions are an issue since they can help to enter some URLs more quickly.

What we don't want, though, are auto-corrected words, nor do we want misspelled words to be underlined for URLs. Bug 823639 was filed because we have "spellcheck enabled", though I don't know exactly what that means.

Aaron, what was the issue you filed bug 823639 for? Were your URL entries being auto-corrected or underlined for misspellings? Or something else?

Comment 10

[Terry]

Well I'm assuming the OP was referring to suggestions since he suggested "textNoSugesstions."

Had I known that we actually wanted suggestions this entire time, this bug would have been moot since day 1 since I would not have been able to reproduce the bug. The only "auto-correct" that happens on my device that I've seen so far is insertion of a space after periods and we didn't even notice that for a while. I have also never seen it underline words for spell-check in the URL areas.

Comment 11

[Aaron Train [:aaronmt]]

(In reply to Brian Nicholson (:bnicholson) from comment #9)
> Aaron, what was the issue you filed bug 823639 for? Were your URL entries
> being auto-corrected or underlined for misspellings? Or something else?

Underlining spell-check (and at the time of filing, dictionary suggestions) on the URL field.

This bug sounds like an exact dupe of bug my bug.

Comment 12

[Terry]

(In reply to Aaron Train [:aaronmt] from comment #11)
> (In reply to Brian Nicholson (:bnicholson) from comment #9)
> > Aaron, what was the issue you filed bug 823639 for? Were your URL entries
> > being auto-corrected or underlined for misspellings? Or something else?
> 
> Underlining spell-check (and at the time of filing, dictionary suggestions)
> on the URL field.
> 

OK, I think you guys need to talk it out to define what the bug actually is because if it involves turning off dictionary suggestions then perhaps the AwesomeBar should be a related bug and the included patch is already a fix. If it doesn't involve removing suggestions, then I can't recreate the bug. In which case, you should probably give more details on how to recreate the underlining spell-check bug since to this date, I do not remember seeing this. Please include devices and software versions used, etc.

> This bug sounds like an exact dupe of bug my bug.

That's because it is... Was it not you who renamed this bug from "Reopened: Text Suggestions for URL's in the Edit Bookmark dialog." to what it is now? That's why I had "Reopened" in the title to begin with and referred to the old bug in my first two posts. The old bug died when nobody replied to this comment: https://bugzilla.mozilla.org/show_bug.cgi?id=823639#c25

Someone on IRC said to just create a new bug, so I did. I fail to see the source of what sounds like confusion...

What exactly is the bug now?

Comment 13

[Brian Nicholson (:bnicholson)]

For the sake of consistency, I think we should just use the same flags we're using for the AwesomeBar (textUri|textNoSuggestions). Even if some phones ignore the "textNoSuggestions" flag, I don't see any major issue as long as the URL isn't auto-corrected or underlined. And if we decide to change that behavior, we should do it for the AwesomeBar too, and that would be beyond the scope of this bug.

Comment 14

[Brian Nicholson (:bnicholson)]

Comment on attachment 740068 [details] [diff] [review]
Added the Java file fix and removed the XML fix that was previously accepted.

r- since the requirements have changed.

Comment 15

[Terry]

Attachment: New patch with XML change only; added textUri

OK, I have made the changes suggested by Brian. This bug no longer concerns decisions regarding whether or not text suggestions are something we want the user to see when typing URLs.

As it stands, the addition of the "textUri" requirement provides a ".com" key and also seems to prevent auto-spacing after periods, so it is a good fix if we make no assumptions about text suggestions.

Hopefully, this will be the last patch for this bug.

Comment 16

[Brian Nicholson (:bnicholson)]

Comment on attachment 745501 [details] [diff] [review]
New patch with XML change only; added textUri

Looks good. Thanks!

Comment 17

[Terry]

Cool. Marking as resolved. I suppose the other issues brought up will have to take form in a new and separate bug if they are to be pursued further.