Closed Bug 1212352 Opened 9 years ago Closed 7 years ago

Add a TV simulator option on WebIDE

Categories

(Firefox OS Graveyard :: Simulator, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED INVALID

People

(Reporter: jj.evelyn, Assigned: jj.evelyn)

References

Details

Attachments

(1 file)

Once bug 1201451 is done, we have a TV simulator available for WebIDE users. Let's make it an option on the UI.
No longer blocks: 1201456, 1020668
No longer depends on: simulator-configuration
I'm happy to help here, but I need someone pointing out the code I need to modify.
Assignee: nobody → ehung
After looking up the code, I guess we should modify the logic here to make it be able to identify a TV xpi file path and its AddonID.

https://dxr.mozilla.org/mozilla-central/source/devtools/client/webide/modules/addons.js#166-177

and also add a record into this json file:
https://ftp.mozilla.org/pub/mozilla.org/labs/fxos-simulator/index.json

@Janx, I guess you are the best person to mentor me on this bug. What's your suggestion of the json format? The minimal change will be something like:
{
  "stable": ["1.3", "1.4", "2.0", "2.1", "2.2"],
  "unstable": ["2.6", "2.6-tv"]
}
but I know it's ugly...

How can I test my patch? Could I build a version of WebIDE separately(i.e. without building a whole package of firefox desktop)?
Flags: needinfo?(janx)
Actually, there is no need to change this logic, because already allows for TV simulators (the "tv" suffix will end up in VERSION). It should be sufficient to just host a new TV Simulator XPI file, and reference it in the JSON file.

Your suggested format makes sense to me:
- As you can see in [0], I already did something similar to reference an empty TV Simulator XPI for tests in [1].
- Since I used "2.6_tv" instead of "2.6-tv", do you mind using a "_" as well?

Also, I think we'll need a "2.6_tv" folder as well similar to [2] (:jryans can help you with that).

[0] https://dxr.mozilla.org/mozilla-central/source/devtools/client/webide/test/addons/simulators.json
[1] https://dxr.mozilla.org/mozilla-central/source/devtools/client/webide/test/addons/
[2] https://ftp.mozilla.org/pub/labs/fxos-simulator/2.6/
Flags: needinfo?(janx)
You can play with the json file by modifying the following pref: devtools.webide.addonsURL
that would allow you to load a custom json file.
Note that you may have to wipe also this pref: devtools.webide.addonsURL_cache
That we use to cache the index.json file.

I'm not sure hacking into the existing index.json like that is ideal.
We are going to mix phone and tv simulators and the only way to distinquish will be this "-tv" or "_tv" in the name.
It would be clearer if content/addons.js was somehow making a clearer distinction between phone and tv builds.
We may hack index.json like you suggested, but it would be great to at least add some visual polish in content/addons.js somehow. (Like putting all TV simulators at the end of the list, or making the addon label looks significantly different)
Attached image show_tv_onWebIDE.png
(In reply to Alexandre Poirot [:ochameau] from comment #4)
> You can play with the json file by modifying the following pref:
> devtools.webide.addonsURL
> that would allow you to load a custom json file.
> Note that you may have to wipe also this pref:
> devtools.webide.addonsURL_cache
> That we use to cache the index.json file.
> 

Cool, I tried and attached the result in comment 5.

> I'm not sure hacking into the existing index.json like that is ideal.
> We are going to mix phone and tv simulators and the only way to distinquish
> will be this "-tv" or "_tv" in the name.
> It would be clearer if content/addons.js was somehow making a clearer
> distinction between phone and tv builds.
> We may hack index.json like you suggested, but it would be great to at least
> add some visual polish in content/addons.js somehow. (Like putting all TV
> simulators at the end of the list, or making the addon label looks
> significantly different)

Yep, the UI looks not so obvious for TV. But I want to unblock here, could we do this visual polish later? (Guess it's a question to Jan. :)
Flags: needinfo?(janx)
(In reply to Jan Keromnes [:janx] from comment #3)
> Actually, there is no need to change this logic, because already allows for
> TV simulators (the "tv" suffix will end up in VERSION). It should be
> sufficient to just host a new TV Simulator XPI file, and reference it in the
> JSON file.
> 

Unfortunately, I got a download failed. Guess it's a file path issue, I'm looking at it.

> Your suggested format makes sense to me:
> - As you can see in [0], I already did something similar to reference an
> empty TV Simulator XPI for tests in [1].
> - Since I used "2.6_tv" instead of "2.6-tv", do you mind using a "_" as well?

Sure, no problem.

> 
> Also, I think we'll need a "2.6_tv" folder as well similar to [2] (:jryans
> can help you with that).
> [2] https://ftp.mozilla.org/pub/labs/fxos-simulator/2.6/

Yep, Alex created one:
https://ftp.mozilla.org/pub/labs/fxos-simulator/2.6-tv/
Yes, let's put the TV Simulator up and do the visual polish in WebIDE later.

(In reply to Evelyn Hung [:evelyn] from comment #7)
> > - Since I used "2.6_tv" instead of "2.6-tv", do you mind using a "_" as well?
> 
> Sure, no problem.

Great, let's use "_" then.

> Yep, Alex created one:
> https://ftp.mozilla.org/pub/labs/fxos-simulator/2.6-tv/

Maybe the download failed issue is because we use "-" instead of "_" here?
Flags: needinfo?(janx)
(In reply to Jan Keromnes [:janx] from comment #8)
> > Yep, Alex created one:
> > https://ftp.mozilla.org/pub/labs/fxos-simulator/2.6-tv/
> 
> Maybe the download failed issue is because we use "-" instead of "_" here?

Yep.. but not just because of the folder name. :(
The actual file path is:
  pub/labs/fxos-simulator/2.6-tv/mac64/fxos_2_6_simulator_tv-mac64-latest.xpi

but the code generates:
  pub/labs/fxos-simulator/2.6_tv/mac64/fxos_2_6_tv_simulator-mac64-latest.xpi


addonID is matched though.
Alex, the fast way to solve this problem is modifying file path on the server. Could you help modifying file name? We prefer underline and let 'tv' token be part of version number.

Sorry to workaround in this way, but I feel it's not worth to write serveral lines of code just for string manipulation. :(
Flags: needinfo?(poirot.alex)
Done. Please review everything as all is very manual work :/
Flags: needinfo?(poirot.alex)
Hi Alex,

Sorry to bother you again, but the filename of Linux64 version should be 'fxos_2_6_tv_simulator-linux64-latest.xpi'. 

Mac and Windows work perfectly. \o/

If you have permission to edit https://ftp.mozilla.org/pub/mozilla.org/labs/fxos-simulator/index.json, could you also update it as the following content?

{
  "stable": ["1.3", "1.4", "2.0", "2.1", "2.2"],
  "unstable": ["2.6", "2.6_tv"]
}

Thank you so much!
Flags: needinfo?(poirot.alex)
BTW, we will update a stable 2.5 version once we finish our polish and bug fixing. :)
Done. Fixed linux64 name and updated the json file.
Please also ensure reviewing the install.rdf files in order to have correct automatic updates.
Flags: needinfo?(poirot.alex)
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: