Closed Bug 844530 Opened 9 years ago Closed 4 years ago

Paper over "JavaScript Error: NS_ERROR_FACTORY_NOT_REGISTERED in BrowserElementPromptService._init()" for B2G

Categories

(Firefox OS Graveyard :: General, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(blocking-b2g:-, firefox20 wontfix, firefox21 wontfix, firefox22 affected, b2g18+ affected, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix)

RESOLVED WONTFIX
B2G C4 (2jan on)
blocking-b2g -
Tracking Status
firefox20 --- wontfix
firefox21 --- wontfix
firefox22 --- affected
b2g18 + affected
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- wontfix

People

(Reporter: justin.lebar+bug, Assigned: justin.lebar+bug)

References

Details

Attachments

(1 file)

bent reported the following JS error in bug 844390:

> E/GeckoConsole(  397): [JavaScript Error: "NS_ERROR_FACTORY_NOT_REGISTERED: Component 
> returned failure code: 0x80040154 (NS_ERROR_FACTORY_NOT_REGISTERED) 
> [nsIComponentManager.getClassObject]" {file: "resource://gre/modules
> /BrowserElementPromptService.jsm" line: 572}]

This is being caused by code we have in place to make <iframe mozbrowser> work properly on desktop as well as b2g.  But I think we can easily paper over the problem specifically on b2g.
Attached patch Patch, v1Splinter Review
Kyle, you understand Javascript...
Attachment #717568 - Flags: review?(khuey)
Assignee: nobody → justin.lebar+bug
I'd love to get this into v1.0.1, but I think that's not likely to happen.  But we should definitely get this into the next version; see bug 844390 comment 0 for a description of the harm this bug causes.
blocking-b2g: --- → leo?
Comment on attachment 717568 [details] [diff] [review]
Patch, v1

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

r=me, but I can't help but fear that we're wallpapering over some toxic mold.
Attachment #717568 - Flags: review?(khuey) → review+
Comment on attachment 717568 [details] [diff] [review]
Patch, v1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): n/a

User impact if declined: Occasionally broken homescreen; see bug 844530 comment 1.
Risk to taking this patch (and alternatives if risky): No alternatives at this time if we want to fix the bug.

String or UUID changes made by this patch: n/a
Attachment #717568 - Flags: approval-mozilla-b2g18?
Backed out for B2G test bustage.
https://hg.mozilla.org/integration/mozilla-inbound/rev/58c7e618c860

https://tbpl.mozilla.org/php/getParsedLog.php?id=20153945&tree=Mozilla-Inbound

12:30:24     INFO -  restarting B2G
12:30:24     INFO -  waiting for system-message-listener-ready...
12:30:24     INFO -  Traceback (most recent call last):
12:30:24     INFO -    File "runreftestb2g.py", line 583, in <module>
12:30:24     INFO -      sys.exit(main())
12:30:24     INFO -    File "runreftestb2g.py", line 503, in main
12:30:24     INFO -      marionette = Marionette.getMarionetteOrExit(**kwargs)
12:30:24     INFO -    File "/home/cltbld/talos-slave/test/build/venv/lib/python2.6/site-packages/marionette/marionette.py", line 196, in getMarionetteOrExit
12:30:24     INFO -      m = cls(*args, **kwargs)
12:30:24     INFO -    File "/home/cltbld/talos-slave/test/build/venv/lib/python2.6/site-packages/marionette/marionette.py", line 171, in __init__
12:30:24     INFO -      busybox=busybox)
12:30:24     INFO -    File "/home/cltbld/talos-slave/test/build/venv/lib/python2.6/site-packages/marionette/emulator.py", line 347, in setup
12:30:24     INFO -      self.wait_for_system_message(marionette)
12:30:24     INFO -    File "/home/cltbld/talos-slave/test/build/venv/lib/python2.6/site-packages/marionette/emulator.py", line 264, in wait_for_system_message
12:30:24     INFO -      """)
12:30:24     INFO -    File "/home/cltbld/talos-slave/test/build/venv/lib/python2.6/site-packages/marionette/marionette.py", line 510, in execute_async_script
12:30:24     INFO -      scriptTimeout=script_timeout)
12:30:24     INFO -    File "/home/cltbld/talos-slave/test/build/venv/lib/python2.6/site-packages/marionette/marionette.py", line 255, in _send_message
12:30:24     INFO -      self._handle_error(response)
12:30:24     INFO -    File "/home/cltbld/talos-slave/test/build/venv/lib/python2.6/site-packages/marionette/marionette.py", line 290, in _handle_error
12:30:24    ERROR -      raise JavascriptException(message=message, status=status, stacktrace=stacktrace)
12:30:24    ERROR -  marionette.errors.JavascriptException: TypeError: navigator.mozKeyboard is null at: app://keyboard.gaiamobile.org/js/keyboard.js line: 384
Comment on attachment 717568 [details] [diff] [review]
Patch, v1

Given comment 3 and comment 6 - not approving this for uplift.
Attachment #717568 - Flags: approval-mozilla-b2g18? → approval-mozilla-b2g18-
(In reply to Lukas Blakk [:lsblakk] from comment #7)
> Comment on attachment 717568 [details] [diff] [review]
> Patch, v1
> 
> Given comment 3 and comment 6 - not approving this for uplift.

We have no way to fix this bug other than this approach.  If you don't approve it for uplift, this bug will remain unfixed, unless someone figures out another approach.

I thought we specifically took low-risk hacks on branches and saved the "right fixes" for m-c?
I think the mistake here is a trivial one.  It doesn't require re-review, so I'm going to ask for approval here on the original patch.  If this updated patch requires large changes, I'll ask for approval on the updated patch.

It's not acceptable to me to have to circle back to every bug that lands on central and ask for approval only at that time.  I'd like release drivers to please pre-approve a safe patch here for landing on b2g18.  Then I don't need to do anything more after I push this patch to inbound, which is how things should be.

I don't want to continue having to argue my point like this; it's a waste your and my time.  I think you guys should and in fact do trust my judgement about landing things on b2g18.
Attachment #717568 - Flags: approval-mozilla-b2g18- → approval-mozilla-b2g18?
Sorry for the bustage; this should work a lot better.

https://hg.mozilla.org/integration/mozilla-inbound/rev/c6c6535fb6c2

I'm going to file a bug on the fact that we're not getting useful error messages out of Marionette.  The error we should have gotten is

> E/GeckoConsole( 4966): [JavaScript Error: "NS_ERROR_XPC_BAD_CONVERT_JS: Could not convert JavaScript 
> argument arg 0 [nsIComponentRegistrar.registerFactory]" {file:"resource://gre/module
> /BrowserElementPromptService.jsm" line: 601}]
Oh, this error is in the marionette logs; it's just buried.  I guess that's good!
This was backed out because we thought it had caused Mn failures. Turns out it didn't. Re-landed (third time's the charm!).
https://hg.mozilla.org/integration/mozilla-inbound/rev/7cdeee48963c
https://hg.mozilla.org/mozilla-central/rev/7cdeee48963c
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Attachment #717568 - Flags: approval-mozilla-b2g18? → approval-mozilla-b2g18+
If it doesn't block v1.0.1, it won't block v1.1. Tracking/approving instead.
blocking-b2g: leo? → -
tracking-b2g18: --- → +
This code is completely wrong and never should have worked.  I have no idea why it worked when I tested it; maybe I wasn't actually updating the files on my phone somehow.

I backed it out.  The backout should resolve bug 848302.  Note that v1.0.1 was unaffected by this bustage.

https://hg.mozilla.org/integration/mozilla-inbound/rev/fdc942a65143
https://hg.mozilla.org/releases/mozilla-b2g18/rev/5a054f4fcafe

Sorry, everyone.  I'll see what it would take to get some test coverage here so we don't regress this.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Depends on: 848302
Comment on attachment 717568 [details] [diff] [review]
Patch, v1

This patch is busted because we never call unregisterFactory on B2G.  The subsequent registerFactory call is not sufficient, afaict.
Attachment #717568 - Flags: review-
https://hg.mozilla.org/mozilla-central/rev/fdc942a65143

Funny story, your commit message has the wrong bug # in it.
https://hg.mozilla.org/mozilla-central/rev/fdc942a65143

Funny story, your commit message has the wrong bug # in it.
Firefox OS is not being worked on
Status: REOPENED → RESOLVED
Closed: 9 years ago4 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.