Closed
Bug 1060294
Opened 10 years ago
Closed 7 years ago
[bluetooth] Update new settings.html with gaia-header
Categories
(Firefox OS Graveyard :: Gaia, defect)
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: wilsonpage, Assigned: yor)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
text/x-github-pull-request
|
Details |
Since our gaia-header patch landed, a new settings.html document was made using the old <header> building block. This needs to be converted to use gaia-header
Reporter | ||
Updated•10 years ago
|
Summary: [bluetooth] Update new setttings.html with gaia-header → [bluetooth] Update new settings.html with gaia-header
Comment 2•10 years ago
|
||
Thank you, Wilson. If we can get review on this on Monday (a holiday in U.S. and Canada) it should still be able to make FL. So close!
Comment 3•10 years ago
|
||
https://github.com/mozilla-b2g/gaia/commit/3c04e67480a2f64b99d7a5cf84071f801f3d9a40
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 4•10 years ago
|
||
Reverted for breaking settings navigation and the settings integration test bluetooth_settings_test.js. Caused failures like this: https://tbpl.mozilla.org/php/getParsedLog.php?id=47112673&tree=B2g-Inbound#error1 master: https://github.com/mozilla-b2g/gaia/commit/2be78d83a760fa3b9638fe51c266b442d14597f1
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 5•10 years ago
|
||
make sure to update the query selector here when relanding: https://github.com/mozilla-b2g/gaia/blob/91d0294120eaa35a60f84fe82c7d89d2e01902cb/apps/settings/test/marionette/app/bluetooth_app.js#L17
Attachment #8481897 -
Flags: review?(mhenretty)
Attachment #8481897 -
Flags: review?(iliu)
Updated Wilson's PR to use gaia-subheader and fixed integration tests.
Attachment #8481280 -
Attachment is obsolete: true
Attachment #8481280 -
Flags: review?(iliu)
Comment 8•10 years ago
|
||
Who did the initial review for this? If I can verify the bluetooth parts got reviewed earlier, I don't mind r+ the test fixes. But I don't see it in this bug :/
Flags: needinfo?(yor)
Flags: needinfo?(wilsonpage)
I'm not sure. Wilson handled the initial patch and he's on PTO until the 8th. I see mikeh landed this on the 30th but I don't know who gave him the go ahead. Stephany, do you know the details or can you track down someone to review this? iliu hasn't responded on this so far.
Flags: needinfo?(yor) → needinfo?(swilkes)
Comment 10•10 years ago
|
||
Comment on attachment 8481897 [details]
Updated PR
Per chat with Wilson before his PTO, assigned to gmarty. Please take this ASAP... it's merge day.
Attachment #8481897 -
Flags: review?(gmarty)
Flags: needinfo?(swilkes)
Comment 11•10 years ago
|
||
And... Guillaume is in London. Michael, you may be our only hope. These time zones are absolutely killing us on web component reviews in particular. :(
Comment 12•10 years ago
|
||
Comment on attachment 8481897 [details]
Updated PR
I can't review this since I have no idea about the bluetooth code :( I'm still unclear why this landed without a bluetooth peer review (or any review at all?). Review might've caught the settings breakage.
Also, we have already branched 2.1, so whenever this does get r+ it's going to need uplift approval as well.
Attachment #8481897 -
Flags: review?(mhenretty)
Comment 13•10 years ago
|
||
Comment on attachment 8481897 [details] Updated PR I'm sorry about to review the patch late because the new architecture of embedded Bluetooth app is broken in these day. Since bug 900551 fixed permission checking, we can not use embedded iframe in settings app in v2.1. It will kill embedded Bluetooth app. Then, kill settings app too. So that bug 1061437 already reverted embedded Bluetooth app in settings app. Cause so much inconvenience, not what we would like to see. In other word, the patch of this bug will not be executed. It will run into settings/elements/bluetooth.html as before. But we still want to re-enabled embedded Bluetooth in v2.2 while nested OOP is ready to support. Then, Yor's patch will be ready in the future while embedded Bluetooth app is re-enabled. Yor, could you please update the patch for the latest code base? Thanks.
Attachment #8481897 -
Flags: review?(iliu)
Comment 15•10 years ago
|
||
Comment on attachment 8481897 [details]
Updated PR
Clearing the review flag for now.
Attachment #8481897 -
Flags: review?(gmarty)
Reporter | ||
Comment 16•10 years ago
|
||
mhenretty: I wrote the initial patch for this and I think due to a communication mixup :mikeh assumed it was ready to land. Apologies, this was not my intention, I was expecting it to go through review first.
Flags: needinfo?(wilsonpage)
Comment 17•7 years ago
|
||
Firefox OS is not being worked on
Status: REOPENED → RESOLVED
Closed: 10 years ago → 7 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•