Closed Bug 1204268 Opened 9 years ago Closed 6 years ago

"appcache validate" command doesn't work when manifest is in outer directory

Categories

(DevTools Graveyard :: Graphic Commandline and Toolbar, defect, P2)

Product:
DevTools Graveyard
Component:
Graphic Commandline and Toolbar
Version:
40 Branch
Type:
defect

Tracking

(firefox60 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 60
Tracking Flags:
Tracking Status
firefox60 --- fixed

People

(Reporter: donrhummy, Assigned: miker, Mentored)

References

Details

(Keywords: good-first-bug, Whiteboard: [good first bug][lang=js])

Attachments

(1 file, 4 obsolete files)

Bug 1204268 - 'appcache validate' command doesn't work when manifest is in outer directory
59 bytes, text/x-review-board-request
miker
: review+
Details 
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:40.0) Gecko/20100101 Firefox/40.0
Build ID: 20150826023504

Steps to reproduce:

Created a page at "http://lampsite.com/some-directory" and linked manifest:

    <html manifest="/wp-content/themes/mytheme/manifest.php?id=124">

Then I typed "shift-F2" to open the dev tools app cache command line and typed "appcache validate".


Actual results:

It showed an error and showed an incorrect URL for the manifest:

    Manifest URI: http://lampsite.com/some-directory/wp-content/themes/mytheme/manifest.php?id=124

    Page does not exist. points to a resource that is not available at line 1.

Notice that the url was put into the page's directory BUT the url in the HTML tag's manifest attribute is at the ROOT of the website (it's prepended with a "/").

IMPORTANT: I am not sure if this is just a bug with the dev tool, or if Firefox itself is incorrectly looking for the manifest there as well.


Expected results:

It should have looked for the manifest at the correct URL:

    http://lampsite.com/wp-content/themes/mytheme/manifest.php?id=124
Component: Untriaged → Developer Tools
Component: Developer Tools → Developer Tools: Graphic Commandline and Toolbar
Summary: Dev Toolbar "shift-F2" "appcache validate" doesn't work when manifest is in outer directory → "appcache validate" command doesn't work when manifest is in outer directory
Status: UNCONFIRMED → NEW
Ever confirmed: true
Mentor: mratcliffe
Whiteboard: [good-first-bug lang=js]
Hi Michael, do we still want to fix this even though appcache is deprecated?
Flags: needinfo?(mratcliffe)
If you could then that would be awesome... although appcache is deprecated a lot of sites and apps still use it.
Flags: needinfo?(mratcliffe)
(In reply to Michael Kohler [:mkohler] from comment #1)
> Hi Michael, do we still want to fix this even though appcache is deprecated?

I would argue it's very important. As Michael points out a *lot* of sites rely on this. While eventually there will be other methods, they are not widely supported yet to be viable solutions. It's likely 2-3 years before those solutions become supported enough that they can be the main method for offline apps. Until then, appcache is critical.
Modified Whiteboard 'good-first-bug' to 'good first bug'.
See https://wiki.mozilla.org/Good_first_bug
Whiteboard: [good-first-bug lang=js] → [good first bug][lang=js]
Hi fellow contributors. I have been thinking about this bug, and so far my high level solution to this bug involves checking whether the manifest path begins with a "/" and acting accordingly.In the case that the manifest begins with a "/", I will determine the orgin of the page and append the manifest path. In the case that the manifest doesn't begin with a "/", I will take the current URL of the page minus the html file and append the manifest path. Now  while looking through documentation for the app cache feature, I found the following:

"The manifest attribute in a web application can specify either the relative path of a cache manifest file or an absolute URL. (Absolute URLs must be from the same origin as the application)".

src = https://developer.mozilla.org/en-US/docs/Web/HTML/Using_the_application_cache


Is this documentation correct? I tested the "appcache validate" command against a html page that has a manifest that is an absolute URL, and it turns out that the dev tool fails to determine the correct URL to the manifest file. For instance, if the html page containing the manifest is served from "http://localhost:3030/dir/app.html" and the manifest path is set to "http://localhost:3030/appcache.manifest", then the "appcache validate" reports the manifest URL as "http://localhost:3030/dir/app.html/http://localhost:3030/appcache.manifest". Am I missing something?
> Am I missing something?

I am sure that the documentation is correct and that the issue is with devtools/client/shared/AppCacheUtils.jsm itself.

_getManifestURI() starts on line 312 and is most likely the source of these bugs.
I'll start working on this.
Assignee: nobody → dalimilhajek
Status: NEW → ASSIGNED
(In reply to Dalimil Hajek [:dalimil] from comment #7)
> I'll start working on this.

Sorry, I should have continued working on the bug sooner. I would appreciate it if you would leave this bug to me. Since I have committed a fair amount of time on it, and I am currently working on writing tests for the patch I wrote.
Assignee: dalimilhajek → nobody
Status: ASSIGNED → NEW
[good first bug] whiteboard -> keyword mass change
Keywords: good-first-bug
I'm running the devtools/client/commandline/test/browser_cmd_appcache_valid.js, which is the test responsible for checking the functionality of the piece of code causing the bug, and the test is timing out on my machine. I have not modified the code that I have forked from mozilla-central, this test is failing right out of the box. I have had several conversations on the beginner and devtools irc channels, but I have not been able to find the root the issue. Other tests in mozilla central work fine for me. The output of the test is hosted here: https://pastebin.mozilla.org/8907229. Could I have access to the Try Server in order to proceed with developing tests for my patch?
If you look at devtools/client/commandline/test/browser.ini in the same folder as the test you can see this in the [DEFAULT] section:
skip-if = e10s # Bug 1034511

This means that all the tests in this folder are disabled in multi-process (e10s) mode.

Rather than wait for the fix you can work around this issue by running the test like this:
./mach mochitest devtools/client/commandline/test/browser_cmd_appcache_valid.js --disable-e10s
Attached patch Possible Bug Fix (obsolete) — — Splinter Review 
The diff marks seem to be off for the devtools/client/commandline/test/browser_cmd_appcache_valid.js file. I tried reverting the file, then modifying the file (i.e. adding a comment) and update the patch to include the revised browser_cmd_appcache_valid.js, but stillthe diff reports more edits than I made.
According to the warning at the top of this article: https://developer.mozilla.org/en-US/docs/Web/HTML/Using_the_application_cache, we should be moving towards Service Workers. Is it okay to pick up this bug and work on it in the light of that?
Flags: needinfo?(mratcliffe)
I think that will be fine... assigning to you.
Assignee: nobody → lenikmutungi
Has STR: --- → yes
Priority: -- → P2
Followed patel's changes. Here are the errors I get when running the test
`./mach mochitest devtools/client/commandline/test/browser_cmd_appcache_valid.js --disable-e10s`
Build configuration changed. Regenerating backend.
Traceback (most recent call last):
  File "/home/user/src/mozilla-central/build/gen_test_backend.py", line 7, in <module>
    from mozbuild.backend.test_manifest import TestManifestBackend
  File "/home/user/src/mozilla-central/build/mach_bootstrap.py", line 327, in __call__
    module = self._original_import(name, globals, locals, fromlist, level)
  File "/home/user/src/mozilla-central/python/mozbuild/mozbuild/backend/test_manifest.py", line 12, in <module>
    from mozbuild.backend.base import PartialBackend
  File "/home/user/src/mozilla-central/build/mach_bootstrap.py", line 327, in __call__
    module = self._original_import(name, globals, locals, fromlist, level)
  File "/home/user/src/mozilla-central/python/mozbuild/mozbuild/backend/base.py", line 28, in <module>
    from ..frontend.data import ContextDerived
  File "/home/user/src/mozilla-central/build/mach_bootstrap.py", line 327, in __call__
    module = self._original_import(name, globals, locals, fromlist, level)
  File "/home/user/src/mozilla-central/python/mozbuild/mozbuild/frontend/data.py", line 24, in <module>
    from .context import FinalTargetValue
  File "/home/user/src/mozilla-central/build/mach_bootstrap.py", line 327, in __call__
    module = self._original_import(name, globals, locals, fromlist, level)
  File "/home/user/src/mozilla-central/python/mozbuild/mozbuild/frontend/context.py", line 40, in <module>
    from ..testing import (
  File "/home/user/src/mozilla-central/build/mach_bootstrap.py", line 327, in __call__
    module = self._original_import(name, globals, locals, fromlist, level)
  File "/home/user/src/mozilla-central/python/mozbuild/mozbuild/testing.py", line 13, in <module>
    from mozpack.copier import FileCopier
  File "/home/user/src/mozilla-central/build/mach_bootstrap.py", line 327, in __call__
    module = self._original_import(name, globals, locals, fromlist, level)
  File "/home/user/src/mozilla-central/python/mozbuild/mozpack/copier.py", line 12, in <module>
    from mozpack.files import (
  File "/home/user/src/mozilla-central/build/mach_bootstrap.py", line 327, in __call__
    module = self._original_import(name, globals, locals, fromlist, level)
  File "/home/user/src/mozilla-central/python/mozbuild/mozpack/files.py", line 33, in <module>
    from jsmin import JavascriptMinify
  File "/home/user/src/mozilla-central/build/mach_bootstrap.py", line 327, in __call__
    module = self._original_import(name, globals, locals, fromlist, level)
ImportError: No module named jsmin
/home/user/src/mozilla-central/build/rebuild-backend.mk:24: recipe for target 'backend.TestManifestBackend' failed
make: *** [backend.TestManifestBackend] Error 1
Error running mach:

    ['mochitest', 'devtools/client/commandline/test/browser_cmd_appcache_valid.js', '--disable-e10s']

The error occurred in code that was called by the mach command. This is either
a bug in the called code itself or in the way that mach is calling it.

You should consider filing a bug for this issue.

If filing a bug, please include the full output of mach, including this error
message.

The details of the failure are as follows:

Exception: Process executed with non-0 exit code 2: [u'/usr/bin/make', u'-f', u'/home/user/src/mozilla-central/build/rebuild-backend.mk', u'-j4', u'-s', u'backend.TestManifestBackend']

  File "/home/user/src/mozilla-central/testing/mochitest/mach_commands.py", line 312, in run_mochitest_general
    tests = mochitest.resolve_tests(test_paths, test_objects, cwd=self._mach_context.cwd)
  File "/home/user/src/mozilla-central/testing/mochitest/mach_commands.py", line 111, in resolve_tests
    resolver = self._spawn(TestResolver)
  File "/home/user/src/mozilla-central/python/mozbuild/mozbuild/base.py", line 623, in _spawn
    topobjdir=self.topobjdir)
  File "/home/user/src/mozilla-central/python/mozbuild/mozbuild/testing.py", line 195, in __init__
    self.topsrcdir, 'build', 'gen_test_backend.py'),
  File "/home/user/src/mozilla-central/python/mozbuild/mozbuild/base.py", line 551, in _run_make
    return fn(**params)
  File "/home/user/src/mozilla-central/python/mozbuild/mozbuild/base.py", line 606, in _run_command_in_objdir
    return self.run_process(cwd=self.topobjdir, **args)
  File "/home/user/src/mozilla-central/python/mach/mach/mixin/process.py", line 147, in run_process
    raise Exception('Process executed with non-0 exit code %d: %s' % (status, args))

Running the test without the --disable-e10s option gives me this error:

Build configuration changed. Regenerating backend.
Traceback (most recent call last):
  File "/home/user/src/mozilla-central/build/gen_test_backend.py", line 7, in <module>
    from mozbuild.backend.test_manifest import TestManifestBackend
  File "/home/user/src/mozilla-central/build/mach_bootstrap.py", line 327, in __call__
    module = self._original_import(name, globals, locals, fromlist, level)
  File "/home/user/src/mozilla-central/python/mozbuild/mozbuild/backend/test_manifest.py", line 12, in <module>
    from mozbuild.backend.base import PartialBackend
  File "/home/user/src/mozilla-central/build/mach_bootstrap.py", line 327, in __call__
    module = self._original_import(name, globals, locals, fromlist, level)
  File "/home/user/src/mozilla-central/python/mozbuild/mozbuild/backend/base.py", line 28, in <module>
    from ..frontend.data import ContextDerived
  File "/home/user/src/mozilla-central/build/mach_bootstrap.py", line 327, in __call__
    module = self._original_import(name, globals, locals, fromlist, level)
  File "/home/user/src/mozilla-central/python/mozbuild/mozbuild/frontend/data.py", line 24, in <module>
    from .context import FinalTargetValue
  File "/home/user/src/mozilla-central/build/mach_bootstrap.py", line 327, in __call__
    module = self._original_import(name, globals, locals, fromlist, level)
  File "/home/user/src/mozilla-central/python/mozbuild/mozbuild/frontend/context.py", line 40, in <module>
    from ..testing import (
  File "/home/user/src/mozilla-central/build/mach_bootstrap.py", line 327, in __call__
    module = self._original_import(name, globals, locals, fromlist, level)
  File "/home/user/src/mozilla-central/python/mozbuild/mozbuild/testing.py", line 13, in <module>
    from mozpack.copier import FileCopier
  File "/home/user/src/mozilla-central/build/mach_bootstrap.py", line 327, in __call__
    module = self._original_import(name, globals, locals, fromlist, level)
  File "/home/user/src/mozilla-central/python/mozbuild/mozpack/copier.py", line 12, in <module>
    from mozpack.files import (
  File "/home/user/src/mozilla-central/build/mach_bootstrap.py", line 327, in __call__
    module = self._original_import(name, globals, locals, fromlist, level)
  File "/home/user/src/mozilla-central/python/mozbuild/mozpack/files.py", line 33, in <module>
    from jsmin import JavascriptMinify
  File "/home/user/src/mozilla-central/build/mach_bootstrap.py", line 327, in __call__
    module = self._original_import(name, globals, locals, fromlist, level)
ImportError: No module named jsmin
/home/user/src/mozilla-central/build/rebuild-backend.mk:24: recipe for target 'backend.TestManifestBackend' failed
make: *** [backend.TestManifestBackend] Error 1
Error running mach:

    ['mochitest', 'devtools/client/commandline/test/browser_cmd_appcache_valid.js']

The error occurred in code that was called by the mach command. This is either
a bug in the called code itself or in the way that mach is calling it.

You should consider filing a bug for this issue.

If filing a bug, please include the full output of mach, including this error
message.

The details of the failure are as follows:

Exception: Process executed with non-0 exit code 2: [u'/usr/bin/make', u'-f', u'/home/user/src/mozilla-central/build/rebuild-backend.mk', u'-j4', u'-s', u'backend.TestManifestBackend']

  File "/home/user/src/mozilla-central/testing/mochitest/mach_commands.py", line 312, in run_mochitest_general
    tests = mochitest.resolve_tests(test_paths, test_objects, cwd=self._mach_context.cwd)
  File "/home/user/src/mozilla-central/testing/mochitest/mach_commands.py", line 111, in resolve_tests
    resolver = self._spawn(TestResolver)
  File "/home/user/src/mozilla-central/python/mozbuild/mozbuild/base.py", line 623, in _spawn
    topobjdir=self.topobjdir)
  File "/home/user/src/mozilla-central/python/mozbuild/mozbuild/testing.py", line 195, in __init__
    self.topsrcdir, 'build', 'gen_test_backend.py'),
  File "/home/user/src/mozilla-central/python/mozbuild/mozbuild/base.py", line 551, in _run_make
    return fn(**params)
  File "/home/user/src/mozilla-central/python/mozbuild/mozbuild/base.py", line 606, in _run_command_in_objdir
    return self.run_process(cwd=self.topobjdir, **args)
  File "/home/user/src/mozilla-central/python/mach/mach/mixin/process.py", line 147, in run_process
    raise Exception('Process executed with non-0 exit code %d: %s' % (status, args))


added devtools/client/commandline/test/browser_cmd_appcache_valid_index_absolute_manifest.html
added devtools/client/commandline/test/browser_cmd_appcache_valid_index_root_relative_manifest.html
changed devtools/client/commandline/test/browser_cmd_appcache_valid.js
changed devtools/client/commandline/test/browser_cmd_appcache_valid_index.html
changed devtools/client/commandline/test/browser_cmd_appcache_valid_page1.html
changed devtools/client/commandline/test/browser_cmd_appcache_valid_page2.html
changed devtools/client/commandline/test/browser_cmd_appcache_valid_page3.html
changed devtools/client/shared/AppCacheUtils.jsm
Comment on attachment 8879982 [details] [diff] [review]
"appcache validate" command doesn't work when manifest is in outer directory

I'm not very sure about how to go about solving this bug, since there isn't much detail to look at in the comments beyond the error report. If you could point me in the direction of any resources that would be helpful, I would be much obliged.
Flags: needinfo?(mratcliffe)
Attachment #8879982 - Flags: review?(mratcliffe)
Attachment #8879982 - Flags: review?(mratcliffe) → feedback?(mratcliffe)
Re-doing Dipen Patel's [:patelndipen] changes on a fresh build of Firefox and adding the requisite files to
browser_cmd_appcache_valid.js as well as browser_cmd_appcache_valid_appcache.appcache
seemed to remove most of the errors. I could try out adding all the files to the
browser_cmd_appcache_valid_appcache.appcache file just to see if that would work but I half-feel
that may not be the right approach since it appears to me that browser_cmd_appcache_valid_appcache.appcache
is supposed to be populated during the test run rather than manually populated. I could be wrong though :P
Running `./mach mochitest devtools/client/commandline/test/browser_cmd_appcache_valid.js --disable-e10s`
fails after an initial successful run during which the `browser_cmd_appcache_valid_index_relative_manifest.html`
page is shown along with its page 1, 2 and 3 HTML files.

added devtools/client/commandline/test/browser_cmd_appcache_valid_index_absolute_manifest.html
added devtools/client/commandline/test/browser_cmd_appcache_valid_index_relative_manifest.html
added devtools/client/commandline/test/browser_cmd_appcache_valid_index_root_relative_manifest.html
changed devtools/client/commandline/test/browser.ini
changed devtools/client/commandline/test/browser_cmd_appcache_valid.js
changed devtools/client/commandline/test/browser_cmd_appcache_valid_appcache.appcache
changed devtools/client/commandline/test/browser_cmd_appcache_valid_page1.html
changed devtools/client/commandline/test/browser_cmd_appcache_valid_page2.html
changed devtools/client/commandline/test/browser_cmd_appcache_valid_page3.html
changed devtools/client/shared/AppCacheUtils.jsm
removed devtools/client/commandline/test/browser_cmd_appcache_valid_index.html
Attachment #8896673 - Flags: review?(mratcliffe)
Attachment #8879982 - Attachment is obsolete: true
Attachment #8879982 - Flags: feedback?(mratcliffe)
(In reply to Leni Kadali [:leni1] [GMT+3] from comment #17)
> Running `./mach mochitest
> devtools/client/commandline/test/browser_cmd_appcache_valid.js
> --disable-e10s`
> fails after an initial successful run during which the
> `browser_cmd_appcache_valid_index_relative_manifest.html`
> page is shown along with its page 1, 2 and 3 HTML files.
By this I mean the hyperlinks to them are visible, not that the pages themselves appear during 
the test :-)
Depends on: 1237782
Hello. Is it possible to get a review for this patch or do I need to rebase it and resubmit it first?
Flags: needinfo?(jwalker)
Mike - I think this is best handled by you.
Flags: needinfo?(jwalker) → needinfo?(mratcliffe)
(In reply to Leni Kadali [:leni1] [GMT+3] from comment #19)
> Hello. Is it possible to get a review for this patch or do I need to rebase
> it and resubmit it first?

As we discussed on IRC, I will review the patch tonight and let you know if there are any further changes you need to make. We will then run the patch through try (our testing servers) to ensure that everything is working properly.
Flags: needinfo?(mratcliffe)
Attachment #8791467 - Attachment is obsolete: true
Attachment #8896673 - Attachment is obsolete: true
Attachment #8896673 - Flags: review?(mratcliffe)
I could not apply your patch to our mozilla central repository so I have taken the opportunity to rebase it and fix the remaining test failures.

We need to wait for the results of the try run and if our tests pass we can land the patch.

Of course, like many GCLI commands, this only works in non E10S mode.
Has Regression Range: --- → irrelevant
OS: Unspecified → All
Hardware: Unspecified → All
The try failures are all unrelated to the appcache changes so I will land this change.
Comment on attachment 8946867 [details]
Bug 1204268 - Make 'appcache validate command work when manifest outside root

https://reviewboard.mozilla.org/r/216752/#review222764
Attachment #8946867 - Flags: review?(mratcliffe) → review+
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s ea6b2d3fd385a4eac58a99e75d0d6b1f55f29018 -d d5600d2ea770: rebasing 445071:ea6b2d3fd385 "Bug 1204268 - Make 'appcache validate command work when manifest outside root r=miker" (tip)
merging devtools/client/commandline/test/browser.ini
merging devtools/client/commandline/test/browser_cmd_appcache_valid.js
warning: conflicts while merging devtools/client/commandline/test/browser_cmd_appcache_valid.js! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
I will rebase this so that it can be landed.
Assignee: lenikmutungi → mratcliffe
Status: NEW → ASSIGNED
Attachment #8946867 - Attachment is obsolete: true
Comment on attachment 8954335 [details]
Bug 1204268 - 'appcache validate' command doesn't work when manifest is in outer directory

https://reviewboard.mozilla.org/r/223438/#review229536
Attachment #8954335 - Flags: review?(mratcliffe) → review+
Pushed by mratcliffe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ff968c3897de
'appcache validate' command doesn't work when manifest is in outer directory r=miker
https://hg.mozilla.org/mozilla-central/rev/ff968c3897de
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox60: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Product: Firefox → DevTools
Product: DevTools → DevTools Graveyard
Depends on: 1619673
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: