Closed Bug 964755 Opened 7 years ago Closed 7 years ago

Failed to open "Connect with WPS" window if there are any known networks with security WPA/WPA2

Categories

(Firefox OS Graveyard :: Wifi, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:1.3+, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed)

RESOLVED FIXED
1.4 S1 (14feb)
blocking-b2g 1.3+
Tracking Status
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed

People

(Reporter: vasanth, Assigned: dwi2)

References

()

Details

(Keywords: regression, Whiteboard: [caf priority: p2][CR 606906][FT:RIL][ETA:2/7])

Attachments

(2 files, 1 obsolete file)

STR:
1. Goto Settings -> WiFi -> Manage Networks -> Join Hidden Network -> 
Enter some name -> Select security type "WPS" -> Click "Ok"
2. Then Turn off and Turn on Wifi
3. Now click "Connect with WPS" 
4. Failed to open "Connect with WPS" and seeing below error.

Error
01-21 10:46:45.150 E/GeckoConsole( 1054): [JavaScript Error: "TypeError: capabilities is undefined" {file: "app://settings.gaiamobile.org/shared/js/wifi_helper.js" line: 147}]

Checking whether network.capabilities is undefined here [1] and returning empty array fixes issue in this use case.
1. However why network.capabilities is always undefined for known networks?
2. Whether in any other case, will it be undefined?

[1] https://github.com/mozilla-b2g/gaia/blob/v1.3/shared/js/wifi_helper.js#L70
Whiteboard: CR606906 → [CR 606906]
ni Ken for Wifi. Given the CNY will continue to find someone locally.
Flags: needinfo?(kchang)
Is this a regression?
Flagging QA to see if we can verify the regression and get a window.
Keywords: regression
blocking-b2g: --- → 1.3?
blocking-b2g: 1.3? → 1.3+
Masashi, because it is the Chinese new year vacation in Taipei now, I wonder if you could help this bug.
Flags: needinfo?(kchang) → needinfo?(masashi.honma)
QA Contact: sparsons
This issue started to occur on the Buri 1.3 Build ID: 20131223004001

Gaia   01e9da49be2cc4bc134eeefc434740d572ec2246
SourceStamp af28fe58e263
BuildID 20131223004001
Version 28.0a2

Last working Buri 1.3 Build ID: 20131222004001

Gaia   5d31cfb04081605a5c4cddd97022ed3210e9bdd0
SourceStamp 6c6fd43f57a4
BuildID 20131222004001
Version 28.0a2
> 1. Goto Settings -> WiFi -> Manage Networks -> Join Hidden Network -> Enter some name ->
> Select security type "WPS" -> Click "Ok"

I could not reproduce this issue.
Because I could not find out option "WPS" on the security selection.
If you want to connect with WPS, you do not need to goto "Manage Networks".

Gaia: aedd5c9636f305d4433491056a0ca984dfb859b1
Gecko: 997a649ab0f536dc3bba78cccb00678f6f97515e
Device: KEON (I don't have Buri)
Flags: needinfo?(masashi.honma)
I can answer this, Yes. You are correct, there is no WPS option on the security screen. I found that selecting any other security option will work as well. I chose WPA-PSK. WEP will work too.

(In reply to Masashi Honma from comment #6)
> > 1. Goto Settings -> WiFi -> Manage Networks -> Join Hidden Network -> Enter some name ->
> > Select security type "WPS" -> Click "Ok"
> 
> I could not reproduce this issue.
> Because I could not find out option "WPS" on the security selection.
> If you want to connect with WPS, you do not need to goto "Manage Networks".
> 
> Gaia: aedd5c9636f305d4433491056a0ca984dfb859b1
> Gecko: 997a649ab0f536dc3bba78cccb00678f6f97515e
> Device: KEON (I don't have Buri)
Bug 937561 looks causes this bug.
Bug 937561 changes to get known networks on scanning.

But I think the modification is not a good idea.
Because user can enter non existing SSID as known network.
And known network is not always available just now.

So the AP list should be displayed based on current scan result.
I attached a PR.
Attached file Pull Request
Attachment #8368364 - Flags: review?(tzhuang)
Attachment #8368364 - Attachment mime type: text/plain → text/html
(In reply to Masashi Honma from comment #6)
> Because I could not find out option "WPS" on the security selection.

That's my bad. Wrongly mentioned between WPA as WPS. Thanks Sarah for clarifying.
Attached patch proposed patch (obsolete) — Splinter Review
I have another proposed patch as in the attachment, only one line changed. Since known networks may not available in the air, it is reasonable to return false when asking isWpsAvailable().

If we backout the patch of bug 937561 as Masashi proposed, then we have to reopen bug 937561. The patch I proposed can solve these two problems at the same time.
Comment on attachment 8368364 [details]
Pull Request

Since I am not module owner, transfer review to Evelyn.
Attachment #8368364 - Flags: review?(tzhuang) → review?(ehung)
Attachment #8368412 - Flags: feedback?(masashi.honma)
Attachment #8368412 - Flags: feedback?(ehung)
I can understand your patch solves both issues and my patch reproduces Bug 937561.
But always showing known network on scan result is not correct approach.
Because user expects the scan result shows "available" network just now.
We also should solve Bug 937561 by another approach.

(In reply to Tzu-Lin Huang [:dwi2][:tzhuang] from comment #11)
> Created attachment 8368412 [details] [diff] [review]
> proposed patch
> 
> I have another proposed patch as in the attachment, only one line changed.
> Since known networks may not available in the air, it is reasonable to
> return false when asking isWpsAvailable().
> 
> If we backout the patch of bug 937561 as Masashi proposed, then we have to
> reopen bug 937561. The patch I proposed can solve these two problems at the
> same time.
Now I changed my mind. The changes for Bug 937561 is OK. But the WiFi icon should be changed if the known SSID is not active.

I watched dwi2's patch for Bug 964755. It looks good.
Attachment #8368412 - Flags: feedback?(masashi.honma)
Inder

Pleae confirm if the issue can be closed from QC?
Flags: needinfo?(ikumar)
Preeti - Sorry, didn't get your comment. The issue is still there and looks like there is an agreement on dwi2's patch.

Vasanth -- can you please try the patch and see if it solves the issue for you?
Flags: needinfo?(ikumar) → needinfo?(vasanth)
(In reply to Inder from comment #16)
> Vasanth -- can you please try the patch and see if it solves the issue for
> you?

dwi2's patch works. Please merge it soon. Thanks for the clarifications!
Flags: needinfo?(vasanth)
Whiteboard: [CR 606906] → [CR 606906][FT:RIL]
Vasanth,

Could we have an ETA for getting this patch landed on 1.3? 
Just mark ETA date on the whiteboard. example-> ETA:2/7
Assignee: vasanth → nobody
(removed Vasanth as the assignee to fix the confusion)
Thanks Masashi Honma's kindly feedback. Currently we display "no signal icon" for known but not-in-the-air wifi networks. I think the icon itself make sense to users.
I made a pull request for my proposed patch and added unit test for it.

Hi Evelyn,
Could you please help to review my patch? Thanks
Attachment #8370529 - Flags: review?(ehung)
Assignee: nobody → tzhuang
I'm confused by 'capability' and 'security'. It seems we mix them since bug 791506, and a comment 'Bug 791506: Code for backward compatibility. Modify after landed.' is added everywhere. Is it kind of an API inconsistency? Are changes in bug 791506 intended to workaround this API inconsistency? If yes, which Gecko bug we are waiting for?

I'm fine to land Tzu-Lin's patch with follow-up bugs filed if it's an API problem.
Comment on attachment 8368364 [details]
Pull Request

clear review flag since the PR has been closed by the author.
Attachment #8368364 - Flags: review?(ehung)
Attachment #8368412 - Flags: feedback?(ehung)
Attachment #8368412 - Attachment is obsolete: true
Comment on attachment 8370529 [details] [review]
pull request for master branch

r+ with my comment on the PR addressed. Thanks!
Attachment #8370529 - Flags: review?(ehung) → review+
ni masashi to answer my question in comment 21. Thanks!
Flags: needinfo?(masashi.honma)
'capability' can include anything about the AP.
'security' includes only security properties in 'capability'.

The codes marked 'Bug 791506: Code for backward compatibility. Modify after landed.' could be erased now.
These are prepared to prevent Gaia and Gecko mismatch.
I will write PR to erase backward compatibility codes after dwi2's PR merged.

(In reply to Evelyn Hung [:evelyn][:喵] from comment #21)
> I'm confused by 'capability' and 'security'. It seems we mix them since bug
> 791506, and a comment 'Bug 791506: Code for backward compatibility. Modify
> after landed.' is added everywhere. Is it kind of an API inconsistency? Are
> changes in bug 791506 intended to workaround this API inconsistency? If yes,
> which Gecko bug we are waiting for?
Flags: needinfo?(masashi.honma)
Whiteboard: [CR 606906][FT:RIL] → [CR 606906][FT:RIL][ETA:2/7]
(In reply to Masashi Honma from comment #25)
> 'capability' can include anything about the AP.
> 'security' includes only security properties in 'capability'.
> 
> The codes marked 'Bug 791506: Code for backward compatibility. Modify after
> landed.' could be erased now.
> These are prepared to prevent Gaia and Gecko mismatch.
> I will write PR to erase backward compatibility codes after dwi2's PR merged.

Thanks for clarify. bug 968050 opens for you. :)
Travis is green now.
https://travis-ci.org/mozilla-b2g/gaia/builds/18252109

Landed on master
https://github.com/mozilla-b2g/gaia/commit/de4eb7cbd60f9ed4d37c5aa1090cc7244f597f98
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
v1.3: de510b2870f281b83e6e10d182197294d8930be3
Target Milestone: --- → 1.4 S1 (14feb)
Flags: in-moztrap?
Flags: in-moztrap? → in-moztrap+
Whiteboard: [CR 606906][FT:RIL][ETA:2/7] → [caf priority: p2][CR 606906][FT:RIL][ETA:2/7]
You need to log in before you can comment on or make changes to this bug.