773884 - Remove special "index.html" path mapping support for app:// scheme

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

+++ 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)]

Attachment: 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?)]

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)]

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?)]

(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)]

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?)]

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)]

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)]

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)]

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)]

(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)]

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)]

Pull request was accepted. Is there anything else for this before it can pass to review?

Comment 13

[Antonio Manuel Amaya Calvo (:amac)]

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é]

I will land it today.

Comment 15

[[:fabrice] Fabrice Desré]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2f69a9df19a3

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2f69a9df19a3