Closed
Bug 1138508
Opened 9 years ago
Closed 9 years ago
Don't initiate NFC sharing when on the browser private window start page
Categories
(Firefox OS Graveyard :: Gaia::System, defect)
Tracking
(b2g-v2.2 verified, b2g-master verified)
VERIFIED
FIXED
2.2 S7 (6mar)
People
(Reporter: arroway, Assigned: kgrandon)
References
Details
(Keywords: polish, Whiteboard: [systemsfe])
Attachments
(3 files)
Gaia-Rev 77609916ca5ab721150fab2b7bc5c37f43ee3a5a Gecko-Rev https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/27ab8aa34201 Build-ID 20150302002504 Version 37.0 Device-Name flame FW-Release 4.4.2 FW-Incremental eng.cltbld.20150302.042655 FW-Date Mon Mar 2 04:27:05 EST 2015 Bootloader L1TC100118D0 STR: 1. launch the Settings app on two phones 2. Turn on NFC on both phones 3. Launch the Browser on phone A 4. Open a private window and stay on the home page (empty by default) 4. Tap the 2 phones Expected result: The private window homepage page should not show shrinking UI because it could not be shared Actual result: The private window homepage page show shrinking UI See also bug 1000678
Updated•9 years ago
|
Whiteboard: [systemsfe]
Updated•9 years ago
|
blocking-b2g: --- → 2.2?
Comment 2•9 years ago
|
||
Had a discussion during standup. We are not blocking the release on it.
Assignee | ||
Comment 3•9 years ago
|
||
I'll look at this.
Assignee: nobody → kgrandon
Status: NEW → ASSIGNED
Comment 4•9 years ago
|
||
Assignee | ||
Comment 5•9 years ago
|
||
Comment on attachment 8573674 [details] [review] [gaia] KevinGrandon:bug_1138508_nfc_sharing_private_landing > mozilla-b2g:master Krzysztof - Any chance you'd be able to review this small NFC patch for me? Thanks!
Attachment #8573674 -
Flags: review?(kmioduszewski)
Comment 6•9 years ago
|
||
The patch looks ok (cool that it prevents not needed IPC), but are we absolutely sure this cannot be fixed on the browser side? I think that like in Bug 1000678, the browser should not be listening for onpeerready if there is no actual content which can be shared. Ben since you worked on Bug 1000678, is it possible to stop NFC sharing on private browsing landing page? Is this complicated?
Flags: needinfo?(bfrancis)
Comment 7•9 years ago
|
||
(In reply to Krzysztof Mioduszewski[:tauzen] from comment #6) > Ben since you worked on Bug 1000678, is it possible to stop NFC sharing on > private browsing landing page? Is this complicated? That was the old browser app, in the new system browser it looks like the conditional is: if (currentApp && currentApp.isBrowser() && currentApp.config.url) https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/nfc_handler.js#L24 The private browser start page (an HTML page in the system app) is implemented differently to the normal browser start page (the search app) so I guess this same conditional doesn't work. Maybe it just needs tweaking?
Flags: needinfo?(bfrancis)
Comment 8•9 years ago
|
||
(In reply to Ben Francis [:benfrancis] from comment #7) > The private browser start page (an HTML page in the system app) is > implemented differently to the normal browser start page (the search app) so > I guess this same conditional doesn't work. Maybe it just needs tweaking? Ok, I wasn't aware that normal browser start page is a different app. So this explains why it's not shrinking. So actually we're not able to tweak anything here, since |NfcHandler| code is fired after shrinking ui is shown and accepted. The patch proposed by Kevin is the best solution.
Comment 9•9 years ago
|
||
Comment on attachment 8573674 [details] [review] [gaia] KevinGrandon:bug_1138508_nfc_sharing_private_landing > mozilla-b2g:master Test needs some fixing. Left comments on github, by mistake directly on commit not in pull request view (sorry for that!).
Attachment #8573674 -
Flags: review?(kmioduszewski)
Assignee | ||
Comment 10•9 years ago
|
||
Comment on attachment 8573674 [details] [review] [gaia] KevinGrandon:bug_1138508_nfc_sharing_private_landing > mozilla-b2g:master Hey Krzysztof - thanks for the review. Those test assertions were totally wrong so nice catch. I've addressed the github comments - could you take another look?
Attachment #8573674 -
Flags: review?(kmioduszewski)
Comment 11•9 years ago
|
||
Comment on attachment 8573674 [details] [review] [gaia] KevinGrandon:bug_1138508_nfc_sharing_private_landing > mozilla-b2g:master Looks good now, thanks!
Attachment #8573674 -
Flags: review?(kmioduszewski) → review+
Assignee | ||
Updated•9 years ago
|
Component: Gaia::Browser → Gaia::System
Keywords: checkin-needed
Updated•9 years ago
|
Keywords: checkin-needed
Comment 12•9 years ago
|
||
Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/52cc47708d378ba0a46ec518b7348ab2518a8e26
Updated•9 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Updated•9 years ago
|
Target Milestone: --- → 2.2 S8 (20mar)
Comment 13•9 years ago
|
||
Should we uplift?
Assignee | ||
Comment 14•9 years ago
|
||
Comment on attachment 8573674 [details] [review] [gaia] KevinGrandon:bug_1138508_nfc_sharing_private_landing > mozilla-b2g:master (In reply to Gregor Wagner [:gwagner] from comment #13) > Should we uplift? It's a small/simple enough patch that I don't see why not. [Approval Request Comment] [Bug caused by] (feature/regressing bug #): Private browser implementation. [User impact] if declined: If two FirefoxOS phones happen to meet in the wild, and they are *both* using NFC *and* one happens to have the private browsing landing page opened *and* the phones touch, one could be given a UI which is not appropriate. [Testing completed]: Manual and unit tests. [Risk to taking this patch] (and alternatives if risky): Low risk, fairy small and contained patch. [String changes made]: None.
Attachment #8573674 -
Flags: approval-gaia-v2.2?(bbajaj)
Updated•9 years ago
|
Attachment #8573674 -
Flags: approval-gaia-v2.2?(bbajaj) → approval-gaia-v2.2+
Comment 15•9 years ago
|
||
1. This problem is verified pass on latest build of Flame 3.0. Verify result: The private window homepage page cannot be shared, it will not show shrinking UI. 2. I have also verified bug 1000678, the verify result is Pass. The Browser Homepage page cannot be shared, it will not show shrinking UI. See attachment: Verify_video.MP4 Rate: 0/10 Flame 3.0 build: (Pass) Build ID 20150315160203 Gaia Revision d4177902b04b8fedcb7df9a30ae6e9677e03d2d4 Gaia Date 2015-03-13 15:58:35 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/af68c9c0e903 Gecko Version 39.0a1 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150315.192711 Firmware Date Sun Mar 15 19:27:22 EDT 2015 Bootloader L1TC000118D0
Updated•9 years ago
|
Comment 16•9 years ago
|
||
v2.2: https://github.com/mozilla-b2g/gaia/commit/af2f4b83c187e0e190526f8c4a32ede4c470131e
status-b2g-v2.2:
--- → fixed
Target Milestone: 2.2 S8 (20mar) → 2.2 S7 (6mar)
Comment 17•9 years ago
|
||
This problem is verified as "pass" on latest build of Flame 2.2 and N5 2.2. Actual result: The Browser Homepage page and private window homepage page cannot be shared, it will not show shrinking UI. See attachment: Verify_Pass_2.2.3gp Rate: 0/5 Device information: Flame 2.2 build (Pass) Build ID 20150705002505 Gaia Revision ea11f422b687a982f0a961c9aea7858066561707 Gaia Date 2015-07-02 23:37:50 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/c0214b4c1ea0 Gecko Version 37.0 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150705.040440 Firmware Date Sun Jul 5 04:04:51 EDT 2015 Bootloader L1TC000118D0 N5 2.2 build (Pass) Build ID 20150705002505 Gaia Revision ea11f422b687a982f0a961c9aea7858066561707 Gaia Date 2015-07-02 23:37:50 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/c0214b4c1ea0 Gecko Version 37.0 Device Name hammerhead Firmware(Release) 5.1 Firmware(Incremental) eng.cltbld.20150705.041046 Firmware Date Sun Jul 5 04:11:05 EDT 2015 Bootloader HHZ12f
Updated•9 years ago
|
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•