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

RESOLVED FIXED in mozilla18

Status

()

Core
Networking
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: briansmith, Assigned: amac)

Tracking

(Blocks: 1 bug)

Trunk
mozilla18
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-kilimanjaro:+, blocking-basecamp:+)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment)

+++ 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.
Component: General → Networking
Blocks: 764194
(Assignee)

Comment 1

5 years ago
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 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.
Attachment #643543 - Flags: review+
(Assignee)

Comment 3

5 years ago
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'.
(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.
(Assignee)

Comment 5

5 years ago
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).
I think it is better to just modify all the manifests, so there's no "magic" in the build process or at runtime.
(Assignee)

Comment 7

5 years ago
Changed the manifests in gaia and added a pull request:

https://github.com/mozilla-b2g/gaia/pull/2609
Assignee: nobody → amac
(Assignee)

Comment 8

5 years ago
Added pull request https://github.com/mozilla-b2g/gaia/pull/2717 to fix a couple of pending issues when building with this patch.
(Assignee)

Comment 9

5 years ago
Keyboard doesn't launch correctly (or at all) either. So it seems more changes are required.
(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
(Assignee)

Comment 11

5 years ago
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.
(Assignee)

Comment 12

5 years ago
Pull request was accepted. Is there anything else for this before it can pass to review?
Status: NEW → ASSIGNED
(Assignee)

Comment 13

5 years ago
I can't change anything on the state of the bug. AFAIK, it's solved. Anything else needed to get it landed?
I will land it today.
https://hg.mozilla.org/integration/mozilla-inbound/rev/2f69a9df19a3
https://hg.mozilla.org/mozilla-central/rev/2f69a9df19a3
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18

Updated

5 years ago
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.