Closed Bug 922141 Opened 11 years ago Closed 11 years ago

Regression: 1Password add-on cannot fill in Aurora (26a) and later

Categories

(Add-on SDK Graveyard :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: jamie, Assigned: irakli)

References

Details

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9) AppleWebKit/537.71 (KHTML, like Gecko) Version/7.0 Safari/537.71

Steps to reproduce:

We have had several users report recently that the 1Password add-on is broken in Aurora. After some investigation, it seems that our code to emit messages to worker ports is no longer working. We have logs immediately before the emit function should be called, but none of our event handlers in the content script fire.

I tried to reproduce in a new add-on to demonstrate the bug, but I wasn't able to reproduce. Here's the relevant sections of code:

	function sendMessageToTab(tab, name, message) {
		if (!tab) tab = tabs.activeTab;
		workersForTab(tab, function(worker) { 
			if (DEBUG_GLOBAL) console.log("[FIREFOX] attempting to emit message"); 
			worker.port.emit("message", { 'name':name, 'message':message }); 
		});
	}
	function workersForTab(tab, callback) {
		var result = [];
		if (DEBUG_FIREFOX) console.log("[FIREFOX] workersForTab: " + tab.url);
		
		result = workers.filter(function(worker, index, workers){
			return worker.tab === tab;
		});
	
		if (callback) result.forEach(callback);
		if (DEBUG_FIREFOX) console.log("[FIREFOX] workersForTab: got " + result.length);
		return result;
	}

Here is log output from a run of 1Password 4. (I'm using the 1Password 4 extension because it's much simpler and because it gets us further down the line than 1Password 3, which has a broken popup.)

	console.log: onepassword: [AGENT] >onmessage: 1341 bytes: <[…snipped JSON.stringify output…]>
	console.log: onepassword: [AGENT] >handleMessage: 'fillItem'
	console.log: onepassword: [FIREFOX] workersForTab: [snipped URL from active tab]
	console.log: onepassword: [FIREFOX] attempting to emit message
	console.log: onepassword: [FIREFOX] workersForTab: got 1
	console.log: onepassword: [AGENT] <handleMessage: 'fillItem'
	console.log: onepassword: [AGENT] <onmessage: 1 ms

1Password 4 is currently in final stages of beta testing. I can provide a link in private for anyone troubleshooting this issue, but like I said it also happens in the current 1Password 3 extension and is noticeably broken there as well.


Actual results:

In current Aurora channel (26a), 1Password simply refuses to fill and cannot properly locate the current tab's URL. This happens in the current 1Password 3 releases as well as the upcoming 1Password 4 extension, which is much less extension code than 1Password 3's.

In 1Password 3, this results in a broken Panel that has no data. In 1Password 4, the data is visible, but cannot be filled. 

In 1Password 4, the extension sends the current URL to the helper to display the data when the user clicks the toolbar button (or Widget). This works fine and displays the data relevant to the current page. But when responding to the keyboard shortcut, the helper sends a message to the add-on to find the current URL. This message flow is broken. In both cases, selecting a login exhibits the broken behavior.


Expected results:

1Password should continue to operate as in 25 and before
(In reply to Jamie Phelps from comment #0)
> User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9) AppleWebKit/537.71
> (KHTML, like Gecko) Version/7.0 Safari/537.71
> 
> Steps to reproduce:
> 
> We have had several users report recently that the 1Password add-on is
> broken in Aurora. After some investigation, it seems that our code to emit
> messages to worker ports is no longer working. We have logs immediately
> before the emit function should be called, but none of our event handlers in
> the content script fire.
>

I have tried to test 1 password 3.9.19 on aurora build right now & it looks like
UI in the panel is not working at all, I can see following output in the console:


11:03:52.844 ReferenceError: sjcl is not defined popup.min.js:8
11:03:52.875 "[DATASOURCE] Database version: 7" global.min.js:306
11:03:53.036 GET http://127.0.0.1:6258/2 [HTTP/1.1 101 Switching Protocols 1ms]
11:03:52.985 "[AGENT] connected to port 6258" global.min.js:323
11:03:53.153 ReferenceError: sjcl is not defined popup.min.js:8


Since source is minifyied it's really hard for me to tell what's going
on there.


> 
> I tried to reproduce in a new add-on to demonstrate the bug, but I wasn't
> able to reproduce.
>

I'm afraid it'll be even harder for me to reproduce the issue without an access
to a source that causes it :( Is there any way we could maybe debug this issue
together ?

>
> Here's the relevant sections of code:
> 
> 	function sendMessageToTab(tab, name, message) {
> 		if (!tab) tab = tabs.activeTab;
> 		workersForTab(tab, function(worker) { 
> 			if (DEBUG_GLOBAL) console.log("[FIREFOX] attempting to emit message"); 
> 			worker.port.emit("message", { 'name':name, 'message':message }); 
> 		});
> 	}
> 	function workersForTab(tab, callback) {
> 		var result = [];
> 		if (DEBUG_FIREFOX) console.log("[FIREFOX] workersForTab: " + tab.url);
> 		
> 		result = workers.filter(function(worker, index, workers){
> 			return worker.tab === tab;
> 		});
> 	
> 		if (callback) result.forEach(callback);
> 		if (DEBUG_FIREFOX) console.log("[FIREFOX] workersForTab: got " +
> result.length);
> 		return result;
> 	}
> 
> Here is log output from a run of 1Password 4. (I'm using the 1Password 4
> extension because it's much simpler and because it gets us further down the
> line than 1Password 3, which has a broken popup.)
>

I guess issue I'm observing is related to broken popup.
 
>
> 	console.log: onepassword: [AGENT] >onmessage: 1341 bytes: <[…snipped
> JSON.stringify output…]>
> 	console.log: onepassword: [AGENT] >handleMessage: 'fillItem'
> 	console.log: onepassword: [FIREFOX] workersForTab: [snipped URL from active
> tab]
> 	console.log: onepassword: [FIREFOX] attempting to emit message
> 	console.log: onepassword: [FIREFOX] workersForTab: got 1
> 	console.log: onepassword: [AGENT] <handleMessage: 'fillItem'
> 	console.log: onepassword: [AGENT] <onmessage: 1 ms
> 

Sorry I'm little confused now, you said earlier issue was that content script event
handlers were not called but last two logs in the above output seem to display that
message handler got a message no ?

> 1Password 4 is currently in final stages of beta testing. I can provide a
> link in private for anyone troubleshooting this issue, but like I said it
> also happens in the current 1Password 3 extension and is noticeably broken
> there as well.
>

You can get in touch with me in private on gozala@mozilla.com. I would like to
help fixing this issue, but right now it's really hard to tell something as v3
seems to fail because of `ReferenceError` to undefiend `sjcl` which does not
seems to be defined in `popup.min.js` also `popup.html` does not seem to load
any other scripts. It's really hard to draw any conclusions from that, maybe you
could provide (simplified code) snippet illustrating how panel code is initialized
which may explain or make issues with v3 reproducible.


As of v4 I guess panel initialization issue is resolved, but there is another issue
of delivering messages. Again it's hard to reason what is going on there, without any
code example, however it reminds me of a Bug 921000 where messages send to content script
in of the panel fail to be delivered if panel's contentURL is changed right before message
is sent. I wonder if 1 password addon also changes panel contentURL and run's into same issue.
You could try patch from that bug to see if it solves issue you're observing in v4.


>
> 
> 
> Actual results:
> 
> In current Aurora channel (26a), 1Password simply refuses to fill and cannot
> properly locate the current tab's URL. This happens in the current 1Password
> 3 releases as well as the upcoming 1Password 4 extension, which is much less
> extension code than 1Password 3's.
> 
> In 1Password 3, this results in a broken Panel that has no data. In
> 1Password 4, the data is visible, but cannot be filled. 
> 
> In 1Password 4, the extension sends the current URL to the helper to display
> the data when the user clicks the toolbar button (or Widget). This works
> fine and displays the data relevant to the current page. But when responding
> to the keyboard shortcut, the helper sends a message to the add-on to find
> the current URL. This message flow is broken. In both cases, selecting a
> login exhibits the broken behavior.

Wait so in v4 it's only broken if keyboard shortcut in used ? Also I'm not sure
what do you refer to when you say "helper" is that some code in the panel ? In
tab's content script ? 

> 
> 
> Expected results:
> 
> 1Password should continue to operate as in 25 and before


I would really like to help, but I need more input in order to be able to do
some progress on this.
Thanks, Gozala. I will email you directly with more information and we can update this thread with further information as available.
Assignee: nobody → rFobic
After collaborating with Jamie on IRC we discovered that this regression is caused by Bug 899388.
1Password extension used  `if (typeof ChromeWindow == "object")` clause to detect that target browser
is firefox. Since `ChromeWindow` has being removed form content windows expression was no longer
true for the content scripts, there for event handlers where not registered.
Sumbitted a Bug 922878 to provide better browser detection.
Blocks: 899388
Depends on: 922878
I ended up changing our `typeof ChromeWindow` code to check the typeof `self` and `self.port`. I still don't like how we're doing the browser detection and have some ideas for making this foolproof in the future, but this restores 1Password's compatibility with Firefox 26. 

I recommend marking as RESOLVED WONTFIX since it's a 1Password change that was needed and my understanding is Bug 899388 is a good change to keep things tidy. (I would mark it myself, but I didn't want to overstep. :))
Status: UNCONFIRMED → RESOLVED
Closed: 11 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.