Opened 10 years ago
Closed 10 years ago
#31687 closed defect (bug) (fixed)
Customizer: fix errors when embedded in a different origin
Reported by: | mattwiebe | Owned by: | ocean90 |
---|---|---|---|
Milestone: | 4.2 | Priority: | normal |
Severity: | normal | Version: | 4.1 |
Component: | Customize | Keywords: | has-patch |
Focuses: | javascript | Cc: |
Description
Changes introduced during the 4.1 cycle create errors when the Customizer is embedded (via iframe) in an origin other than wp-admin. On WordPress.com, we use https://wordpress.com/customize/yourblogname.wordpress.com
as the route into loading the Customizer in an iframe. This worked fine with the stock Customizer before 4.1 but two changes were needed as of 4.1.
In customize-base.js
, using _.isEqual()
in api.Value::set
produces a SecurityError
, traced back to setting self.targetWindow
in customize-controls.js
, since _.isEqual()
, when checking for equality, attempts to access an object's prototype chain. The blank window object in the iframe (foo.wordpress.com/wp-admin/customize.php) takes on the origin of the parent context (wordpress.com) and attempting to access any object properties produced a fatal SecurityError
, crippling the app and preventing it from loading. The try/catch pattern in the attached patch isn't pretty, but it works.
In customize-controls.js
, the attempt to set the parent's document.title
likewise produced a SecurityError
, but this one at least doesn't cripple the Customizer, it just litters the console with SecurityError
s. Again, try/catch can take care of this.
The patch we're using to alleviate these issues on wp.com is attached.
Attachments (5)
Change History (17)
#2
@
10 years ago
- Milestone changed from Awaiting Review to Future Release
For document.title
we should have just been using postMessage
to begin with, eh?
An alternative for patching wp.customize.Value.set
would be to just override the method for just targetWindow
, and to eliminate the use of _.isEqual()
in that Value's specific set
method.
#3
@
10 years ago
An alternative for patching
wp.customize.Value.set
would be to just override the method for justtargetWindow
Yeah, that's a good idea. 31687.1.diff does just that. Cleaner. :)
#4
@
10 years ago
For document.title we should have just been using postMessage to begin with, eh?
Yep, the framework's in place, let's use it. 31687.2.diff does that. Goodbye nasty try/catch :)
This ticket was mentioned in Slack in #core-customize by mattwiebe. View the logs.
10 years ago
#6
follow-up:
↓ 7
@
10 years ago
@ocean90, @westonruter, do you think this and #31742 would be something we could do in 4.2?
#7
in reply to:
↑ 6
;
follow-up:
↓ 8
@
10 years ago
- Keywords commit added
- Milestone changed from Future Release to 4.2
- Owner set to ocean90
- Status changed from new to reviewing
Replying to obenland:
@ocean90, @westonruter, do you think this and #31742 would be something we could do in 4.2?
I haven't reviewed #31742 yet, but the changes here are great. To be clear, the changes to be committed in this ticket are contained in two separate patches: 31687.1.diff & 31687.2.diff
#8
in reply to:
↑ 7
@
10 years ago
Replying to westonruter:
the changes to be committed in this ticket are contained in two separate patches: 31687.1.diff & 31687.2.diff
I combined those two patches into 31687.3.diff for convenience.
#10
@
10 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
Apologies for apparently not knowing how to test my own code, but this still can create SecurityError
s.
31687.4.diff fixes this by ensuring that targetWindow
always has its set
method overridden before a window
object could wind up in there.
An alternative approach would just be not using
wp.customize.Value
for thetargetWindow
property. I don't think any of the sugar there is needed that badly. This would remove the try/catch overhead for every singleapi.Value
instance in the Customizer.For the
document.title
part, we could do a try/catch initially and remember whether it's an accessible origin or not, sidestepping the try/catch every time.