mozilla :: #content

18 Sep 2012
11:10smaugkhuey: about fsapi?
11:10smaugtime to get rid of it :)
11:11smaugat least try to
12:08petervMs2ger: why the second hunk in ?
12:08firebotBug nor, --, mozilla18, Ms2ger, RESO FIXED, Fix a few bugs in CGDOMJSProxyHandler_defineProperty
12:09Ms2ger"Because bz told me to"
12:09petervthis is the case where we have a named getter, but not a named setter
12:10petervseems like if creating is true then we need to do the defineProperty, but you made us return early?
12:11petervMs2ger: you want me to ask bz?
12:11Ms2gerLet me look
12:11Ms2gerThis code hurts my head :)
12:12petervwell, the interaction between setters/creators/expandos hurts mine ;-)
12:14Ms2gerThat makes you and me both :)
12:16petervMs2ger: so what that block does is we have only a getter, it checks if the getter returns something for the name (which is the only way we have right now to check for "supported property names")
12:17petervMs2ger: if it is a supported property name it throws
12:17petervMs2ger: and if not it used to fall through to the base defineProperty which sets an expando
12:17Ms2gerActually, the spec says
12:17Ms2ger"If creating is false and O does not implement an interface with a named property setter, then Reject."
12:18Ms2gerWhere "Reject" only throws in strict mode...
12:18petervok, that's a separate issue
12:18petervyou can file that
12:18Ms2gerAlso, heycam's if chains confuse me
12:19heycamReject should return if it's not strict mode
12:19heycamthat's a bug if it Reject isn't defined to do that
12:20heycam(it should be just like the "Reject" in the ES spec)
12:21petervMs2ger: or should we not fall through to step 4?
12:21* peterv doesn't understand what should happen then
12:22Ms2gerheycam, yes, that's an orthogonal issue
12:24Ms2gerheycam, so, in
12:24Ms2gerIf you have a named getter, but no named setter
12:25Ms2gerWhat should happen if creating (from step 3.1) is true?
12:27heycamMs2ger, ignored if not strict mode, throw if strict mode
12:28heycamso you are not able to set expandos on objects with named properties
12:28heycameither there is a setter and you invoke that, or there is no setter and you can't set other properties on the object
12:28Ms2gerWhere does the spec say that?
12:30heycamReject is defined to return if not strict mode, throw if it is strict mode
12:30heycamand "creating" is true if you're setting a property that doesn't correspond to a current named property
12:30petervthe spec doesn't say that
12:30Ms2gerI don't see where we're hitting a Reject, though
12:31Ms2gerIf creating is false and ...
12:31petervcreating is true
12:31heycamok my mistake
12:31heycamit looks like it might fall through to step 4
12:32heycamnow I wonder if that's what I wanted to do :)
12:32petervluckily we had a test for this :-P
12:32petervbut it's for the nodeList binding, so we don't hit the bug yet
12:33heycamI see
12:33heycamI should have put a comment in the spec source to remind myself what I actually wanted to do ;)
12:33heycamI think preventing expandos would be simpler, if we can do that
12:33Ms2gerAryeh's test?
12:33heycamI think 3.2.1 maybe should say "If creating is true"?
12:34heycamoh no
12:34heycammisreading again
12:34petervwe can implement whatever, I implemented the spec as is
12:34Ms2gerSep 04 19:25:08 <bz> yes
12:34Ms2gerSep 04 19:25:12 <bz> we should have a return true there
12:34Ms2gerSep 04 19:25:15 <bz> good catch
12:34* Ms2ger implemented what bz said
12:34petervand then Ms2ger &quot;fixed&quot; it
12:34heycamok so bz wants to prevent expandos?
12:35Ms2gerUnless he was confused too :)
12:35heycamI think that&#39;s what I want
12:35heycamso please file a bug!
12:36heycam(and I will try to read public-script-coord some time before the end of the year)
12:37petervbz posted about some issues he saw with platform array types, we need to get that sorted out too
12:37Ms2gerheycam, you know what would be nice? A direct link to file a new bug in your component, like has at the top :)
12:44bakuMs2ger, I read the comment you wrote about the patch for 761227
12:44bakubut that patch has already been landed
12:44Ms2gerI know
12:44bakushould I open a new bug? or create a new patch? and reopen the bug...
12:46Ms2gerFeel free to fix it if you&#39;re around that code some day, and try to remember it the next time you need an nsIDocument from a window
12:46Ms2gerNo need to fix it right now
12:46heycamMs2ger, sure I can add a link for that. file a bug for it? :D
12:50* Ms2ger kicks heycam
12:50Ms2gerAnd filed
12:54Ms2gerI guess bug 792012 should not be in Firefox :: Untriaged
12:54firebotBug nor, --, ---, nobody, UNCO, violates javascript single-threadedness, open multiple windows with same name
13:12petervbz: ping
14:10bholleybz: yt?
14:11Ms2gerThat&#39;s two
14:26smaugbz: ping
14:27smaugoh, is there a ping queue ?
14:28Ms2gerThat&#39;s the queue for pings in this channel, at least :)
14:43smaughmm, this isn&#39;t quite as simple as I hoped...
14:53bzsmaug: ack
14:53bzbholley: ack
14:53bzpeterv: ack
14:53bzthose pings were a while ago....
14:55smaugbz: so...
14:55smaugbz: I&#39;d like to have a way to say that xbl anon content is effectively native anon
14:55smaugthe obvious way to just flag it so doesn&#39;t work because layout doesn&#39;t seem to be able to handle that
14:56smaugother, should-be-rather-simple, approach is to add a new flag
14:56smaugsomething like chromeInContent
14:56smaugit would be set also when something is marked native anon
14:57smaugthen in event handling and domclassinfo we&#39;d use that flag
14:58smaugbut, if it is very easy to just fix layout, that would be the optimal solution
14:58smaugany comments
14:58* smaug hasn&#39;t yet figured out what in layout doesn&#39;t handle this all
14:58bzsmaug: &quot;effectively&quot; in what sense?
14:59smaugso that content can&#39;t access the anon content
14:59smaugand event handling (mutation events etc) work like with native anon
14:59smauglayout should work like with normal XBL
14:59bzcan chrome access native anon content?
15:00bzright now?
15:00bzthen why can&#39;t we just change whatever security check is involved to check for anon instead of native anon?
15:00bzsimilar for event handling?
15:01bzI assume what you tried was setting the &quot;native anon&quot; flag on it or something?
15:01smaugyeah, I tried setting nativeanon flag
15:02smaugevent handling should be changed only for content
15:02smaugwe want to propagate mouseover/out events in chrome when happening in anon content
15:03smaugso, perhaps adding a new flag for this all would be ok
15:03smaugthat bindings in content are quite special
15:03bzThat seems like the safest course of action to me
15:03* smaug tries
15:04bzgood luck...
15:08petervbz: it looks like our TreatNonCallableAsNull works on the attribute, but not on a callback type?
15:08petervbz: adding it to the type isn&#39;t hard, but the question is when to enforce the nullability
15:09petervbz: would finish be the right spot?
15:10petervfinish for the attribute
15:10bzpeterv: checking
15:10petervthe IDLAttribute
15:10bzfinish for the IDLAttribute is a good place for checks of all sorts
15:10petervok, I&#39;ll move the nullability check there
15:11bzbtw, about that named getter/setter/creator thing there was the irc discussion about
15:11bzI _thought_ that what I told ms2ger to do matched the spec
15:11bzbut maybe I misread it
15:11bzthe spec is completely incomprehensible
15:11petervyeah, but apparently the spec is wrong :-)
15:11petervor at least it&#39;s not what the author thinks the intent was
15:11Ms2ger+1 on incomprehensible
15:11petervso I&#39;ll fix the test
15:12petervas part of the patch to turn on the list bindings
15:12petervbz: also, we should find a way to expose the webidl constants to C++ code
15:13bznon-string ones should be easy
15:13bzstrings are a bitch
15:13petervat least non-string ones
15:13* peterv files a bug
15:22* smaug needs to decide the name for the new flag... NATIVE_ANONYMOUS_XBL
15:23Ms2gerToo short
15:25smaugah, indeed
15:25smaughow did I make such mistake
15:26smaugbut I don&#39;t like _XBL
15:26smaugperhaps CHROME_ONLY_CONTENT
16:31mccr8bholley: out of curiousity, are these Components.interfaces.nsIFoo things just some kind of constant that can be forged after your patch, or are they some kind of C++ or whatever thing?
16:31bholleymccr8: I don&#39;t understand
16:32bholleyComponents.interfaces.nsIFoo are of type nsJSIID
16:32bholleymccr8: they&#39;re XPCWNs
16:32bholleymccr8: what do you mean &quot;forged&quot;?
16:32mccr8bholley: well, if it was just some string containing the UUID or whatever, then somebody could still QI to one of the interfaces after you remove C.i.
16:33mccr8bholley: but if it is some kind of XPCWN then I guess not.
16:33bholleymccr8: yeah, it&#39;s the key to QI
16:34bholleymccr8: see CallMethodHelper::QueryInterfaceFastPath
16:34mccr8bholley: so it shouldn&#39;t be possible to QI to those interfaces after your patch, even if you happen to know the UIID or whatever of that interface. okay great just making sure. :)
16:35bholleymccr8: hmm, that&#39;s a clever attack strategy ;-)
16:35bholleymccr8: so, the implementation here is xpc_JSObjectToID
16:35mccr8yeah I was noticing that.
16:35bholleymccr8: so, content could theoretically implement itself as such an object (a double-wrapped XPCOM component), but doing so would requireCi
16:36bholleymccr8: so I think we&#39;re safe
16:36mccr8bholley: hah, okay, good. just checking.
16:36bholleymccr8: I also think we should be defense-in-depth about this stuff if possible though
16:37bholleymccr8: so, for example, right now Components is protected by 2 security checks
16:37bholleymccr8: one is a CAPS thing that I&#39;d like to remove, but am uncomfortable doing so
16:37bholleymccr8: and the other is gabor&#39;s same-compartment security wrapper
16:37bholleymccr8: once we do this XBL thing, we&#39;ll have a third measure, and I&#39;ll be more comfortable removing the CAPS thing
16:37mccr8sounds good!
16:38bholleymccr8: because theoretically, XBL can still leak the Components object, just by doing |window.Components = Components|
16:38bholleywe should probably tell AMO reviewers to watch out for that kind of thing
16:39bholleymccr8: thankfully, they probably _can&#39;t_ be doing that right now, because Components is a permanent property
16:40bholleymccr8: though, we could try putting a little runtime abort in the Components_interfaces getter of XPCComponents that asserts we&#39;re either in system or XBL code
16:40bholleymccr8: not completely sure we could get away with it, but it would be a great idea
16:41mccr8yeah that sounds like a good ide.a
16:41bholleymccr8: or even in QueryInterface, who knows. Wherever it&#39;s cleanest
16:41bholleymccr8: though, darn, that would break our test suite :-(
16:42bholleyle sigh
16:42mccr8let&#39;s just get rid of tests
16:42mccr8it would also speed up try runs
16:45petervbz: hmm, we don&#39;t actually support TreatNonCallableAsNull in the codegen, right?
16:46bzI don&#39;t think we do yet, no
16:46petervwould it be ok to remove the parser support for it?
16:46petervit&#39;s pretty broken
16:47bzwe could implement it in codegen....
16:47petervit rejects valid WebIDL but it&#39;s complicated to fix
16:47bzwhat does it reject that it shouldn&#39;t?
16:48peterv[TreatNonCallableAsNull] callback Function = any (); typedef Function? NullableFunction; ... attribute NullableFunction foo;
16:49petervI also don&#39;t understand how it works with typedef types if one does
16:50* bz unwinds that
16:50bzso why does the parser reject that?
16:50petervFunction is not nullable
16:50petervit enforces nullability in the wrong place
16:50* bz reads spec
16:51petervbut also, it sets the TreatNonCallableAsNull bit on the type of an attribute
16:51peterveven though the keyword is specified only on that attribute
16:51petervit seems to me that tha leaks it to other users of that type
16:51bzso per spec
16:51bzthis extended attr is only relevant on callback functions
16:52bzand spec says:
16:52bzIf the [TreatNonCallableAsNull] extended attribute appears on a callback function, then it indicates that any value assigned to an attribute whose type is a nullable callback function that is not a callable object will be converted to the null value.
16:52* bz looks at what our parser is doing with this
16:53bzSo the bit really belongs with the IDLCallbackType
16:53bzfor whether it had the annotation
16:53bzand the code in IDLType is just bogus
16:54bzIs that making any sense?
16:54petervwell, I guess so
16:55bzSo I would be in favor of:
16:55bz1) Fixing the parser support
16:55bz2) Making codegen throw on uses of callbacks that are not nullable and TreatNonCallableAsNull, since we don&#39;t support anything else
16:55petervok, I&#39;ll try to fix it up
16:55bzI can write the patches if you prefer
16:56petervok, fine
16:56petervit blocks Node
16:56petervthrough EventTarget
16:57petervgtg, bbl
16:59* bz goes to look at this stuff
17:20* Ms2ger loves when someone gets four reviews for a nsContentUtils change, and none of them is a DOM peer
17:23Ms2gerBug 716575
17:23firebotBug nor, P4, ---, mbrubeck, ASSI, Use the native Gecko support for <meta viewport> instead of handling it in the front end code
17:23bzI trust roc on this stuff
17:24Ms2gerSo do I
17:24khueyyeah I don&#39;t think that&#39;s a big deal
17:24Ms2gerBut still, a lot of stuff seems to land without dom peer review lately, and it&#39;s not all roc
17:34smaugsicking: mounir: are you going to tpac?
17:35sickingsmaug: yeah, we&#39;re both going
17:35mounirsicking: still up to do halloween at TPAC? :)
17:36sickinghah, yeah, that would be awesome :)
17:37smaugsicking: webapps and dap?
17:37* mounir should register
17:37mounirsmaug: there is system level api wg meetings?
17:37sickingsmaug: yeah, i think so
17:37smaugI was thinking to participate webapps and there will be possibly something speech related
17:43gaborcan someone help me how to use talos? so I tried to run something with a bunch of talos flags:
17:43Ms2gergabor, #ateam, talk to jhammel or jmaher
17:44gaborMs2ger: thanks
17:49smaugwho is vyang ?
17:49* smaug never knows who is capable to review b2g stuff
17:49Ms2gerThey do reviews?
17:49smaugb2g, sure
17:50smaugdon&#39;t know about gaia
17:54khueysmaug: he&#39;s an engineer in the taipei office
18:25Ms2gerhumph, btw, did you pick things from the list of projects you asked for a while back?
18:41khueydom window utils is a shithole
18:44Ms2gerNews at 11
18:46khueyon a related note
18:46khueywho wants to sign up to review my patches that split nsPIDOMWindow into separate interfaces for inner and outer windows?
18:47Ms2gerI want to see it
18:47Ms2gerBut mrbkap gets to review
18:47khueywell it&#39;s only about half done
18:58humphMs2ger: yeah
19:02smaugmccr8: bah, how did I manage to make that typo
19:02mccr8I&#39;m just glad you updated the documentation.
19:07jdm!summon sicking
20:03smaugok, who doesn&#39;t have a huge review queue
20:04khueysmaug doesn&#39;t!
20:07khueynsFocusManager is very strange code
20:08smaugI doubt I should review my own patches
20:08smaugand yes, focus handling is somewhat odd, but focusmanager made it about 100x easier to understand than what it was before
20:09dougtkhuey: r- if you don&#39;t have docs on those interfaces.
20:12khueydougt: I don&#39;t see your name on :-P
20:13dougtdon&#39;t threaten me with a wiki.
20:14dougtkhuey: but, yo, srsly... if i think everyone outside of #content is confused by inner/outer
20:15khueysome of the people inside #content are confused by it
20:16dougtkhuey: and... it will make jlebar really happy (for a moment).
20:16jlebardougt, please make me happy.
20:17dougthere is the best i could come up with...
20:17dougtjlebar: how&#39;s this... the feature freeze date was just a joke. we were missing with you.. ha ha ha.
20:17jlebardougt, I wasn&#39;t referring to the fleeting moment of happiness as I experience weightlessness, plunging downward off the George Washington bridge.
20:18jlebarSorry, maybe jokes about that are in bad taste.
20:18dougtjlebar: j/k... get back to work.
20:18dougtyeah, probably poor taste, but with this sense of doom..
20:19khueydougt: untangling this stuff is so much fun :-/
20:20khueydougt: like how the focus manager wants the inner window exactly half the time and the outer window exactly half the time
20:22dougtkhuey: after you said the focus manager, i had flashbacks of untangling thread from my moms sewing machine.
20:22dougti so wanted to throw the machine out the window.
20:23dougtkhuey: oh, great... accessible has their own focusmanager too?
20:23smaugkhuey: I actually blame the whole outer/inner window mess
20:23smaugwe really should have separate classes for those
20:23smaugwould make code easier to understand
20:24dougtsmaug: separate interfaces or classes?
20:24khueydougt: separate classes
20:24smaugwell, both
20:24khueysmaug: I have half-done patches to split nsPIDOMWindow apart
20:24khueywhich is by far the worst of hte interfaces
20:27dougti wrote a bunch of dom apis that took the mWindow (maybe an innerWindow) that was on the navigator and then called GetCurrentInnerWindow() on it and stored that result. I was assured that was the window where my new dom api was being used in. I also had tests like if the outerWindow was no longer non-null or if the window didn&#39;t have a docshell, bail.
20:28dougti had a bit more than a passing understanding of how these things work, but i always worried that the code wasn&#39;t entirely correct.
20:28dougti hope the interface/class splitting will help with the usage.
20:29dougtbecause right now, it feels like it is sharp and it is going to cut someone.
20:29mrbkapkhuey: I&#39;ll gladly review that.
20:29mrbkapdougt: it&#39;s cut us several times.
20:30khueyI have to finish it first :-P
20:30mrbkapkhuey: the problem I suspect you&#39;ll run into is that we have APIs that return one or the other depending on randomness.
20:30khueymrbkap: yeah, I found a bunch of fun stuff like that
20:30khueyif (foo)
20:30khuey DoThingReturningInner
20:31khuey return ThingThatHappensToBeOuter
20:31khueyit&#39;s quite fun
20:31mrbkapkhuey: nsIDocument::GetScriptGlobalObject is particularly evil.
20:31mrbkapkhuey: I had most of a patch to remove it, but never got it green on try.
20:32khueyI haven&#39;t done nsDocument.cpp yet
20:32khueyit&#39;s one of the big things left
20:37geekboysicking: ping
20:43sickinggeekboy: pong
20:43geekboyso all apps have an nsDocumment, right?
20:43sickinggeekboy: at least one yes
20:43geekboyso, they all have an nsIChannel too, right?
20:43sickingthat i&#39;m less sure about
20:44sickingi mean, apps have them as much as webpages do
20:44sickingbut i&#39;m not sure that all webpages do
20:44geekboybut you are pretty sure that not all apps will have an nsIHttpChannel?
20:46geekboysorry, that was poorly worded...
20:46geekboymy guess is that not all apps have an affiliated nsIHttpChannel
20:46geekboyand just wanted to know what you thought
20:47sickingmany apps will not have an nsIHttpChannel. That is correct
20:47sickingin particular, packaged apps won&#39;t
21:03mccr8oh ReparentWrapperIfFound, my old friend...
21:04bzyou have odd friends
21:42bholleygeekboy: ping
21:44geekboybholley: pong
21:44bholleygeekboy: are you familiar with the sandboxed iframe spec?
21:45geekboyvery little
21:45geekboyimelven: ^^
21:45bholleygeekboy: who is?
21:45bholleyoh, perfect. I was just responding to imelven in a bug
22:00imelvenbholley: hi
22:00bholleyimelven: almost done commenting in the bug - se
22:00imelvencool, thanks for the help :)
22:01bholleyimelven: commented - let me know if that makes sense
22:04imelvenbholley: it makes sense, except i had &#39;allow-same-origin&#39; set in my test case
22:04bholleyimelven: oh? it doesn&#39;t appear to be in the one attached to the bug. But that would explain why you were in the other compartment
22:04bholleyimelven: more to the point, doing that kind of invalidates any security guarantees around the sandbox stuff ;-)
22:05imelvenyeah, i was using a separate one, sorry
22:05imelvenwell, even with &#39;allow-same-origin&#39; the sandboxed document shouldn&#39;t be able to navigate top (this is also what webkit and ie10 have implemented, i believe)
22:06bholleyimelven: at least, not until it removes the sandbox attribute from its iframe ;-)
22:07bholleyimelven: but yeah, sure
22:28smaugoh, khuey is running out of this todo list
22:29khueythe problem is what to do *next*
22:29khueynot what to do
22:29khueyI have plenty of stuff to do
19 Sep 2012
No messages
Last message: 4 years and 340 days ago