491219 - Playing ogg video will not update page titles on other open tabs

Status: VERIFIED FIXED

(Core :: XPConnect, defect)

Description

[Tony Chung [:tchung]]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090502 Minefield/3.6a1pre
Build Identifier: 

When a ogg video is playing, switching to another open tab will not update the page title of that tab.   The page of the ogg video persists across all other tabs.

Reproducible: Always

Steps to Reproduce:
1. install branch and trunk nightly
2. open a few different URLs (eg. cnn.com, msnbc.com, planet.mozilla.org)
3. open another tab with a ogg video (eg. URL above)
4. on the blizzard video demo site, play the first video in the list
5. as the video plays, navigate to the other tabs 
6. Verify the other tabs has the same page title as the video (eg. Minefield)
7. Verify stopping the video and going to other tabs, will show the responsive page title for that tab
Actual Results:  
Video playback page persists the page title across all tabs

Expected Results:  
Video playback page should not persist the page title across all tabs

Comment 1

[Ria Klaassen (not reading all bugmail)]

Regression range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1a1611bb1063&tochange=e167d6ca2023
I tested this with two tabs, the first was the homepage and the second the URL with the video.

Comment 2

[Ria Klaassen (not reading all bugmail)]

It's also in Shiretoko.

Comment 3

[Justin Dolske [:Dolske]]

Interesting. This seems to be focus related... If you switch tabs immediately after clicking play (or any other part of the video controls), the bug happens. If you click play, and then somewhere on the document outside the <video>, the window title is correctly updated.

Comment 4

[Justin Dolske [:Dolske]]

Aha. So, the immediate problem is in tabbrowser.xul's updateCurrentBrowser()...

854                 var elem = this.mCurrentBrowser.focusedElement;
855                 if (elem instanceof HTMLElement || elem instanceof XULElement) {
856                   elem.blur();
857                 }

When blur() is called, it throws:

which ends up throwing: [Exception... "Unexpected error"  nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame :: chrome://browser/content/tabbrowser.xml :: updateCurrentBrowser :: line 853"  data: no]

I'm going to go out on a limb an assume this yet again has something to do with native anonymous content, but I didn't look into why the blur() fails. From the legends and rumors of focus code I've heard, I'm a little afraid. :-)

I suppose the simple fix would be to just wrap a try/catch around the call. That's enough to fix the title problem, but I notice there's one other side effect... After switching back to the tab with the video, the video controls don't fade back in. The video control <handler> never gets the event, so I assume focus is confused. If I click on the page content, that's enough to get things working again.

Comment 5

[Justin Dolske [:Dolske]]

Enn: ideas? Should we bandaid this with a try/catch, or fix the underlying focus problem?

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This is pretty bad, but it should be easy to at least work around for 1.9.1.

Comment 7

[Neil Deakin]

Justin, the fix for this is likely to just do the else clause that calls nsIDOMWindowUtils::Focus(null) when the call to blur fails.

Comment 8

[Tony Chung [:tchung]]

Also, I noticed it only happens with video controls are shown and video is playing.  If you hide controls, yet still play the video with focus on the video, and switch tabs, the page titles are updated accordingly.

Comment 9

[Tony Chung [:tchung]]

More information here.  This seems to screw up any new tabs and the source of the page.   For example, if you open a new tab during video play, because the page title is retaining the video page data, it's also bringing up the viewsource for the video page instead of the content new page you opened.  

Further artifacts in this state, show if you try and visit a URL in the new page (with the video playing in the other tab), then the new url will open on both the current tab, AND the video tab (video stops, url launches)

Comment 10

[Justin Dolske [:Dolske]]

Attachment: Patch v.1

Enn: so, this makes the document title update correctly, but it doesn't help with the problem described at the end of comment 4. If I explicitly null out this.mCurrentBrowser.focusedElement, the latter issue is resolved as well.

Comment 11

[Justin Dolske [:Dolske]]

Attachment: Patch v.2

This fixes both problems, but I'm unsure of it's evilness.

Also nulled out commandDispatcher.focusedElement, since it looks like the 2 are supposed to be kept in sync, although I couldn't notice any difference without doing that... Nulling mCurrentBrowser.focusedElement is both necessary and sufficient to fix the observed problem.

Comment 12

[:Gavin Sharp [email: [e-mail]]]

You shouldn't set document.commandDispatcher.focusedElement, that actually tries to set focus. Falling back to the alternate nsIDOMWindowUtils.focus(null) call per Neil's comment 7 should also avoid the need to null out this.mCurrentBrowser.focusedElement, I think.

Comment 13

[Justin Dolske [:Dolske]]

Well, the problem is that falling back to the else-block doesn't help -- see Patch v.1 and comment 10... The addition of a try/catch is enough to make the window tile update properly, but when you switch back to the video the controls don't see mouseover/mouseout events.

Comment 14

[:Gavin Sharp [email: [e-mail]]]

Oh, I see. Presumably the whatToFocus.focus() call is throwing when you switch back? That would explain why nulling out focusedElement would avoid it.

Comment 15

[Blake Kaplan (:mrbkap) (inactive)]

This is *so* mine.

Comment 16

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: wip

Dolske tested this patch and it works. I need to clean it up and add some more comments tomorrow. Basically what's happening is that we have a SOW(XPCNW(element)). We resolve the 'blur' function on the element onto the SOW (which is an XPCNativeWrapped function), not wrapping it. We then call the function with 'this' being a SOW. XPC_NW_FunctionWrapper doesn't expect this and throws. The fix is to wrap native functions (not scripted functions, more on that tomorrow) in a SOW function wrapper that unwraps the SOW.

Comment 17

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: patch v1

Turned out to be comments only.

Comment 18

[Ria Klaassen (not reading all bugmail)]

Also, when the tab with the video is not the focused tab and I type an URL, the URL opens over the video in the inactive tab. :)

Comment 19

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/mozilla-central/rev/4a32b3101121

Comment 20

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/01b3b37b1919

Comment 21

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified fixed on trunk and 1.9.1 with builds on all platforms like;

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090601 Minefield/3.6a1pre ID:20090601031227

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090601 Shiretoko/3.5pre ID:20090601031153