Last Comment Bug 781320 - Forward name attribute down through remote <iframe mozbrowser>
: Forward name attribute down through remote <iframe mozbrowser>
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla17
Assigned To: Justin Lebar (not reading bugmail)
:
:
Mentors:
: 766873 (view as bug list)
Depends on: 780351
Blocks: 766873
  Show dependency treegraph
 
Reported: 2012-08-08 12:53 PDT by Justin Lebar (not reading bugmail)
Modified: 2013-04-04 13:53 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Part 1: Sync iframe between parent and child in <iframe mozbrowser>. (2.97 KB, patch)
2012-08-09 08:23 PDT, Justin Lebar (not reading bugmail)
bugs: review+
Details | Diff | Splinter Review
Part 2: Tests for part 1 (5.85 KB, patch)
2012-08-09 08:23 PDT, Justin Lebar (not reading bugmail)
bugs: review+
Details | Diff | Splinter Review
Part 3: Set <iframe mozbrowser>'s name for iframes created via window.open. (3.54 KB, patch)
2012-08-09 08:27 PDT, Justin Lebar (not reading bugmail)
bugs: review+
Details | Diff | Splinter Review
Part 4: Test for part 3. (1.26 KB, patch)
2012-08-09 08:27 PDT, Justin Lebar (not reading bugmail)
bugs: review+
Details | Diff | Splinter Review

Description Justin Lebar (not reading bugmail) 2012-08-08 12:53:03 PDT
In bug 766873, Tim wants to be able to do

  <iframe mozbrowser remote=true mozapp="..." name="background">

In order for that to work, we have to forward the name attribute down into the child process.
Comment 1 Tim Guan-tin Chien [:timdream] (please needinfo; OOO until Oct 17) 2012-08-09 03:35:21 PDT
(In reply to Justin Lebar [:jlebar] from comment #0)
> In bug 766873, Tim wants to be able to do
> 
>   <iframe mozbrowser remote=true mozapp="..." name="background">
> 
> In order for that to work, we have to forward the name attribute down into
> the child process.

So this is the propose fix? I can update Gaia with this first before Gecko fix if you need it.
Comment 2 Justin Lebar (not reading bugmail) 2012-08-09 08:23:09 PDT
Created attachment 650567 [details] [diff] [review]
Part 1: Sync iframe between parent and child in <iframe mozbrowser>.
Comment 3 Justin Lebar (not reading bugmail) 2012-08-09 08:23:21 PDT
Created attachment 650568 [details] [diff] [review]
Part 2: Tests for part 1
Comment 4 Justin Lebar (not reading bugmail) 2012-08-09 08:27:27 PDT
Created attachment 650569 [details] [diff] [review]
Part 3: Set <iframe mozbrowser>'s name for iframes created via window.open.

I was originally concerned that part 1 would break things without the change in this patch.  I was afraid that window.open(..., name) would create an iframe with no name, and then part 1 would read the empty name attribute off the iframe and set the docshell's name to ''.

But in fact, that to set docShell.name = '' code runs /before/ the window watcher comes in and sets the docshell's name to |name|.  So everything worked ok.  (This behavior is tested by the OpenNamed test.)

However, I thought it would still be nice if we'd send |name| up to the iframe.  We're not watching for changes to this attribute to push them down to the child; I don't think that's particularly important.
Comment 5 Justin Lebar (not reading bugmail) 2012-08-09 08:27:42 PDT
Created attachment 650570 [details] [diff] [review]
Part 4: Test for part 3.
Comment 6 Justin Lebar (not reading bugmail) 2012-08-09 08:33:19 PDT
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TW) from comment #1)
> So this is the propose fix? I can update Gaia with this first before Gecko
> fix if you need it.

I don't have a preference as to whether you update Gaia before or after this change.  I do care whether these changes work for you, though: Want to try my git branch?  https://github.com/jlebar/mozilla-central/tree/mozbrowser-open-named
Comment 7 Justin Lebar (not reading bugmail) 2012-08-09 14:45:11 PDT
I can land once we get bug 777135 cleared up.  Currently the tests here die because of some rotten BrowserElementParent.js code removed in that bug.
Comment 9 Tim Guan-tin Chien [:timdream] (please needinfo; OOO until Oct 17) 2012-08-14 07:34:54 PDT
(In reply to Justin Lebar [:jlebar] from comment #6)
> (In reply to Tim Guan-tin Chien [:timdream] (MoCo-TW) from comment #1)
> > So this is the propose fix? I can update Gaia with this first before Gecko
> > fix if you need it.
> 
> I don't have a preference as to whether you update Gaia before or after this
> change.  I do care whether these changes work for you, though: Want to try
> my git branch? 
> https://github.com/jlebar/mozilla-central/tree/mozbrowser-open-named

I'll try that tomorrow. Thanks.
Comment 10 Justin Lebar (not reading bugmail) 2012-08-14 07:38:06 PDT
> I'll try that tomorrow. Thanks.

The changes are in m-c, so should be in tomorrow's nightly.
Comment 11 Tim Guan-tin Chien [:timdream] (please needinfo; OOO until Oct 17) 2012-08-14 20:48:34 PDT
(In reply to Justin Lebar [:jlebar] from comment #10)
> > I'll try that tomorrow. Thanks.
> 
> The changes are in m-c, so should be in tomorrow's nightly.

That works. Thanks. I will close bug 766873 as dup of this one.
Comment 12 Tim Guan-tin Chien [:timdream] (please needinfo; OOO until Oct 17) 2012-08-14 20:52:39 PDT
*** Bug 766873 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.