Forward name attribute down through remote <iframe mozbrowser>

RESOLVED FIXED in mozilla17

Status

()

Core
DOM
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: Justin Lebar (not reading bugmail), Assigned: Justin Lebar (not reading bugmail))

Tracking

Trunk
mozilla17
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

(Assignee)

Description

5 years ago
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.
(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.
(Assignee)

Comment 2

5 years ago
Created attachment 650567 [details] [diff] [review]
Part 1: Sync iframe between parent and child in <iframe mozbrowser>.
Attachment #650567 - Flags: review?(bugs)
(Assignee)

Comment 3

5 years ago
Created attachment 650568 [details] [diff] [review]
Part 2: Tests for part 1
Attachment #650568 - Flags: review?(bugs)
(Assignee)

Comment 4

5 years ago
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.
Attachment #650569 - Flags: review?(bugs)
(Assignee)

Comment 5

5 years ago
Created attachment 650570 [details] [diff] [review]
Part 4: Test for part 3.
Attachment #650570 - Flags: review?(bugs)
(Assignee)

Comment 6

5 years ago
(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
(Assignee)

Updated

5 years ago
Assignee: nobody → justin.lebar+bug

Updated

5 years ago
Attachment #650567 - Flags: review?(bugs) → review+

Updated

5 years ago
Attachment #650568 - Flags: review?(bugs) → review+

Updated

5 years ago
Attachment #650569 - Flags: review?(bugs) → review+

Updated

5 years ago
Attachment #650570 - Flags: review?(bugs) → review+
(Assignee)

Comment 7

5 years ago
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 8

5 years ago
https://hg.mozilla.org/mozilla-central/rev/0b51cea790e1
https://hg.mozilla.org/mozilla-central/rev/1cb5ff3b90b1
https://hg.mozilla.org/mozilla-central/rev/01ad2dc0e9bf
https://hg.mozilla.org/mozilla-central/rev/6e12afd7b4bb
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
(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.
(Assignee)

Comment 10

5 years ago
> I'll try that tomorrow. Thanks.

The changes are in m-c, so should be in tomorrow's nightly.
(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.
Duplicate of this bug: 766873
Component: DOM: Mozilla Extensions → DOM
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.