Closed Bug 795715 Opened 7 years ago Closed 7 years ago

Remove B2G-specific stuff from nsIDOMWindow

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla19
blocking-basecamp -
Tracking Status
firefox18 + fixed
firefox19 --- fixed

People

(Reporter: mounir, Assigned: mounir)

References

Details

Attachments

(1 file)

We should have a B2G-specific DOMWindow interface that would be inherited when building for B2G. That way, we do no expose to the entire Web stuff that can't actually be used.

We should make sure that lands before the next uplift.
This doesn't affect B2G so it's not a Basecamp blocker.  But it should still be done :)
blocking-basecamp: ? → -
(In reply to Mounir Lamouri (:mounir) from comment #0)
> We should have a B2G-specific DOMWindow interface that would be inherited
> when building for B2G. That way, we do no expose to the entire Web stuff
> that can't actually be used.
> 
> We should make sure that lands before the next uplift.

We are a couple of weeks from FF18 being on Aurora to get to beta . As per comment 1 this does not seem to be a blocker, so wanted to check if there is WIP on this else I can go an untrack this for 18's release .
This is IMO a blocker for Firefox release.
Attached patch PatchSplinter Review
Attachment #678742 - Flags: review?(bugs)
Comment on attachment 678742 [details] [diff] [review]
Patch

I don't like the change to domclassinfo. couldn't you just 
ifdef b2g interface?

someone from b2g team should review this too, especially confvars


r+ with domclassinfo change.
Attachment #678742 - Flags: review?(bugs) → review+
Comment on attachment 678742 [details] [diff] [review]
Patch

Fabrice, could you review the b2g bits?
Attachment #678742 - Flags: review?(fabrice)
Comment on attachment 678742 [details] [diff] [review]
Patch

Kyle, I think it would be better to get a quick feedback from you given that I'm messing with build stuff.
Attachment #678742 - Flags: feedback?(khuey)
Comment on attachment 678742 [details] [diff] [review]
Patch

Review of attachment 678742 [details] [diff] [review]:
-----------------------------------------------------------------

::: b2g/confvars.sh
@@ +44,5 @@
>  MOZ_SYS_MSG=1
>  
>  MOZ_PAY=1
>  MOZ_TOOLKIT_SEARCH=
> +MOZ_B2G=1

Do you need that *and* the AC_DEFINE(MOZ_B2G) in configure.in ?
Attachment #678742 - Flags: review?(fabrice) → review+
Comment on attachment 678742 [details] [diff] [review]
Patch

Review of attachment 678742 [details] [diff] [review]:
-----------------------------------------------------------------

ifdefs on interfaces are pretty shitty (on nsPIDOMWindow), but I guess this is ok for b2g.

::: dom/interfaces/base/Makefile.in
@@ +22,5 @@
>  	nsIDOMWindowUtils.idl			\
>  	$(NULL)
>  
> +ifdef MOZ_B2G
> +SDK_XPIDLSRCS += \

Don't use SDK_XPIDLSRCS, just use XPIDLSRCS.
Attachment #678742 - Flags: feedback?(khuey) → feedback+
(In reply to Fabrice Desré [:fabrice] from comment #8)
> Comment on attachment 678742 [details] [diff] [review]
> Patch
> 
> Review of attachment 678742 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: b2g/confvars.sh
> @@ +44,5 @@
> >  MOZ_SYS_MSG=1
> >  
> >  MOZ_PAY=1
> >  MOZ_TOOLKIT_SEARCH=
> > +MOZ_B2G=1
> 
> Do you need that *and* the AC_DEFINE(MOZ_B2G) in configure.in ?

IIRC, this is the common pattern.
https://hg.mozilla.org/integration/mozilla-inbound/rev/236980577c86

The following things happen following IRC discussions:
- Olli's reviews became a super-review;
- Kyle's feedback became a rewiew;
- Olli's comment has been ignored because it wasn't taking into account that class info is set trough a macro (which makes #ifdef inside it not doable).
Flags: in-testsuite-
Target Milestone: --- → mozilla19
https://hg.mozilla.org/mozilla-central/rev/236980577c86
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Comment on attachment 678742 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 795136 and bug 714358
User impact if declined: it's more a Web impact than a user impact. Stuff will be added to Window interface and to the global scope when we could prevent that.
Risk to taking this patch (and alternatives if risky): risks are low for non-B2G given that we disable features that were already not working. For B2G, there are a small risks that the features might no longer work but that would be detected very quickly.
String or UUID changes made by this patch: nsIDOMWindow.idl because it gets some attributes removed.
Attachment #678742 - Flags: approval-mozilla-aurora?
Attachment #678742 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.