Closed Bug 1362047 Opened 7 years ago Closed 7 years ago

Borderify webextension is broken

Categories

(WebExtensions :: General, defect, P1)

Product:
WebExtensions
Component:
General
Version:
55 Branch
Type:
defect

Tracking

(firefox-esr52 unaffected, firefox53 unaffected, firefox54 unaffected, firefox55 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox-esr52 --- unaffected
firefox53 --- unaffected
firefox54 --- unaffected
firefox55 --- fixed

People

(Reporter: vtamas, Assigned: zombie)

References

Details

(Keywords: regression, Whiteboard: [content scripts], investigating)

Attachments

(4 files)

borderify.zip
1.84 KB, application/x-zip-compressed
Details
Bug 1362047 - Add comment explaining unobvous uses of mockAppInfo()
59 bytes, text/x-review-board-request
kmag
: review+
Details
Bug 1362047 - Fix default value for content script run_at, add tests
59 bytes, text/x-review-board-request
Details
Bug 1362047 - Add comment explaining unobvous uses of mockAppInfo()
59 bytes, text/x-review-board-request
kmag
: review+
Details
Attached file borderify.zip — 
[Affected versions]:
Firefox 55.0a1 (2017-05-04)

[Affected platforms]:
Windows 10 64-bit
Ubuntu 16.04 32-bit 
Mac OS X 10.12.1


[Steps to reproduce]:
1.Launch Firefox with a clean profile.
2.Install via about:debugging the attached webextension.
3.Navigate to https://www.mozilla.org/en-US/.

[Expected Results]:
A solid border is added to all webpages matching mozilla.org.


[Actual Results]:
There is no border displayed on mozilla.org pages.

[Regression Range]:
Last good revision: 9bd131795d81977c1d21c9c4aff06b3f4c69956e
First bad revision: 895164016ae91086fe04e1f5cafbaa3e46f3805d
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=9bd131795d81977c1d21c9c4aff06b3f4c69956e&tochange=895164016ae91086fe04e1f5cafbaa3e46f3805d

Looks like the following bug has the  changes which introduced the regression:
https://bugzilla.mozilla.org/show_bug.cgi?id=1317697
Flags: needinfo?(kmaglione+bmo)
andym investigating and will share with kmag if repro
Flags: needinfo?(kmaglione+bmo) → needinfo?(amckay)
Whiteboard: [content scripts], investigating
Assignee: nobody → kmaglione+bmo
Priority: -- → P1
Flags: needinfo?(amckay)
Assignee: kmaglione+bmo → tomica
Status: NEW → ASSIGNED
Comment on attachment 8866118 [details]
Bug 1362047 - Add comment explaining unobvous uses of mockAppInfo()

https://reviewboard.mozilla.org/r/137716/#review141232

Setting "default": "document_idle" in manifest.json should/might fix this, but otherwise r+
Attachment #8866118 - Flags: review?(mixedpuppy) → review+
Attachment #8866118 - Flags: review+ → review?(mixedpuppy)
Switched to using default values in schema, added test for tabs.executeScript.
Comment on attachment 8866118 [details]
Bug 1362047 - Add comment explaining unobvous uses of mockAppInfo()

https://reviewboard.mozilla.org/r/137716/#review142118

LGTM.  Did default need to be set in both places?
Attachment #8866118 - Flags: review?(mixedpuppy) → review+
Comment on attachment 8866118 [details]
Bug 1362047 - Add comment explaining unobvous uses of mockAppInfo()

https://reviewboard.mozilla.org/r/137716/#review142118

Yes, `default` (like `optional`) is a field on the schema Property/Parameter definition, not on the Type itself.
Keywords: checkin-needed
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 22e373fd2ae1 -d c9732a51552b: rebasing 395639:22e373fd2ae1 "Bug 1362047 - Fix content_script.run_at default value, add test r=mixedpuppy" (tip)
merging browser/components/extensions/test/browser/browser_ext_tabs_executeScript_runAt.js
merging toolkit/components/extensions/test/xpcshell/test_ext_contentscript.js
warning: conflicts while merging toolkit/components/extensions/test/xpcshell/test_ext_contentscript.js! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/3397cb95fb95
Fix content_script.run_at default value, add test r=mixedpuppy
Keywords: checkin-needed
Flags: needinfo?(tomica)
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/02c060b287ff
Fix default value for content script run_at, add tests r=mixedpuppy
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/02c060b287ff
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Attachment #8869830 - Flags: review?(mixedpuppy)
Attachment #8866118 - Flags: review?(kmaglione+bmo)
Comment on attachment 8866118 [details]
Bug 1362047 - Add comment explaining unobvous uses of mockAppInfo()

https://reviewboard.mozilla.org/r/137716/#review144924

::: toolkit/components/extensions/test/xpcshell/test_ext_contentscript.js:8
(Diff revision 7)
>  const server = createHttpServer();
>  server.registerDirectory("/data/", do_get_file("data"));
>  
>  const BASE_URL = `http://localhost:${server.identity.primaryPort}/data`;
>  
> +// ExtensionContent.jsm needs to know when it's running from xpcshell.

Please also mention that this is so it can execute document_idle scripts with a reasonable timeout.
Attachment #8866118 - Flags: review?(kmaglione+bmo) → review+
Attachment #8869836 - Flags: review?(kmaglione+bmo)
Comment on attachment 8869836 [details]
Bug 1362047 - Add comment explaining unobvous uses of mockAppInfo()

https://reviewboard.mozilla.org/r/141386/#review144926
Attachment #8869836 - Flags: review?(kmaglione+bmo) → review+
Pushed by maglione.k@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/1ec7b0cb2bfd
Add comment explaining unobvous uses of mockAppInfo() r=kmag
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: