Last Comment Bug 773884 - Remove special "index.html" path mapping support for app:// scheme
: Remove special "index.html" path mapping support for app:// scheme
Status: RESOLVED FIXED
[qa-]
:
Product: Core
Classification: Components
Component: Networking (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla18
Assigned To: Antonio Manuel Amaya Calvo (:amac)
:
: Patrick McManus [:mcmanus]
Mentors:
Depends on:
Blocks: b2g-updates 769350
  Show dependency treegraph
 
Reported: 2012-07-13 17:33 PDT by Brian Smith (:briansmith, :bsmith, use NEEDINFO?)
Modified: 2012-08-29 17:21 PDT (History)
19 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
+


Attachments
Removed the URL modification that added index.html (1.16 KB, patch)
2012-07-18 13:14 PDT, Antonio Manuel Amaya Calvo (:amac)
brian: review+
Details | Diff | Splinter Review

Description Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2012-07-13 17:33:32 PDT
+++ This bug was initially created as a clone of Bug #769350 +++

The patch in bug 769350 added support for mapping files named "some/path/index.html" to "some/path/", emulating what Apache and other web servers do. I talked with Jonas and we both agreed we should avoid such path mapping. This bug is about removing said mapping.
Comment 1 Antonio Manuel Amaya Calvo (:amac) 2012-07-18 13:14:08 PDT
Created attachment 643543 [details] [diff] [review]
Removed the URL modification that added index.html

Is this all that is required (just removed the part that adds index.html to the path), or do you want any special treatment of directory URLS?
Comment 2 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2012-07-18 13:30:02 PDT
Comment on attachment 643543 [details] [diff] [review]
Removed the URL modification that added index.html

Review of attachment 643543 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, this is what I meant.

We need to make sure this change doesn't break any Gaia apps.
Comment 3 Antonio Manuel Amaya Calvo (:amac) 2012-07-18 17:11:32 PDT
I would say that currently it breaks at least the homescreen app (and through it all the rest I guess). I think homescreen use the manifest 'launch_path' property to launch the apps. And currently it's '/' on all the apps. Or '/#whatever'.
Comment 4 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2012-07-18 17:18:43 PDT
(In reply to Antonio Manuel Amaya Calvo from comment #3)
> I would say that currently it breaks at least the homescreen app (and
> through it all the rest I guess). I think homescreen use the manifest
> 'launch_path' property to launch the apps. And currently it's '/' on all the
> apps. Or '/#whatever'.

How hard is it to mass-change that all to "/index.html"? Seems like a simple change to me.
Comment 5 Antonio Manuel Amaya Calvo (:amac) 2012-07-18 17:22:47 PDT
Not that hard. Could either do that or change the Makefile to check that value and add a index.html if it's a / or /#whatever (basically what the JS did, only just on make time). I can make that change without any problem on the gaia Makefile (or on all the manifests, whichever you prefer)... but I don't know how to make a change request for that. 

I can ask around here tomorrow (2.20 AM here now :) ) though. Just let me know which change you prefer (make gaia to add the 'index.html' magically to the incorrect manifests, or modifying all the manifests once and publishing the change).
Comment 6 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2012-07-18 21:25:57 PDT
I think it is better to just modify all the manifests, so there's no "magic" in the build process or at runtime.
Comment 7 Antonio Manuel Amaya Calvo (:amac) 2012-07-19 02:34:34 PDT
Changed the manifests in gaia and added a pull request:

https://github.com/mozilla-b2g/gaia/pull/2609
Comment 8 Antonio Manuel Amaya Calvo (:amac) 2012-07-23 10:12:23 PDT
Added pull request https://github.com/mozilla-b2g/gaia/pull/2717 to fix a couple of pending issues when building with this patch.
Comment 9 Antonio Manuel Amaya Calvo (:amac) 2012-07-23 10:16:11 PDT
Keyboard doesn't launch correctly (or at all) either. So it seems more changes are required.
Comment 10 Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) 2012-07-23 10:29:40 PDT
(In reply to Antonio Manuel Amaya Calvo from comment #9)
> Keyboard doesn't launch correctly (or at all) either. So it seems more
> changes are required.

The keyboard issue is likely in Gaia/apps/system/js/keyboard_manager.js
Comment 11 Antonio Manuel Amaya Calvo (:amac) 2012-07-23 10:36:55 PDT
Ok, I modified the pull request to add the changes to the keyboard (our comments crossed :) ).  As far as I can test it, everything seems to work. 

Note that I don't know most of the apps so 'as far as I can test it' =~ 'the homescreen shows, the keyboard works and all app launch correctly' which isn't saying much :)

Please let me know if something else is needed.
Comment 12 Antonio Manuel Amaya Calvo (:amac) 2012-07-26 09:20:29 PDT
Pull request was accepted. Is there anything else for this before it can pass to review?
Comment 13 Antonio Manuel Amaya Calvo (:amac) 2012-08-29 03:00:40 PDT
I can't change anything on the state of the bug. AFAIK, it's solved. Anything else needed to get it landed?
Comment 14 [:fabrice] Fabrice Desré 2012-08-29 04:03:13 PDT
I will land it today.
Comment 15 [:fabrice] Fabrice Desré 2012-08-29 05:30:13 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/2f69a9df19a3
Comment 16 Ryan VanderMeulen [:RyanVM] 2012-08-29 17:20:30 PDT
https://hg.mozilla.org/mozilla-central/rev/2f69a9df19a3

Note You need to log in before you can comment on or make changes to this bug.