Jason Smith [:jsmith] Reporter
	Description • 11 years ago

See the discussion on bug 859554 for context. Any WebAPI that is not supported on a platform should return undefined, not null. null is only returned if the API is supported on the platform, but the web content does not have access to that API.

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 1 • 11 years ago

I selectively clicked a few depending bugs of bug 859554, only this one is nominated for tef+. Is this on purpose?

	Jason Smith [:jsmith] Reporter
	Comment 2 • 11 years ago

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #1)
> I selectively clicked a few depending bugs of bug 859554, only this one is
> nominated for tef+. Is this on purpose?

Nope. I forgot to clear it. Thanks for the reminder.

Technically the important piece is to fix this for Android and Desktop, not B2G, so this won't block.

	Mounir Lamouri (:mounir)
	Comment 4 • 11 years ago

This is going to be pretty simple to do when WebSMS will be converted to WebIDL.

	Mounir Lamouri (:mounir)
	Comment 5 • 11 years ago

Hmm, it might be Navigator actually.

	Boris Zbarsky [:bzbarsky]
	Comment 6 • 11 years ago

I believe the patches in bug 838146 do this, in fact.  Please double-check that?

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 7 • 11 years ago

This patch basically put everything related to MobileMessage into MOZ_WEBSMS_BACKEND preprocessor section.  The only exception is IPDL files because they're required even WebSMS is disabled.  No convenient way known to avoid them.

Like my bug 889898 comment 4, some of other components have a global switch to completely turn off their functions by removing them from build targets.  It's really nice to have that for WebSMS as well so that we can avoid creating problems, overheads on platforms that don't have WebSMS.  Besides, we may also skip fixing these avoidable issues when we're running out of time to deliver mandatory solution for B2G only.

	Boris Zbarsky [:bzbarsky]
	Comment 8 • 11 years ago

The navigator parts of this look like they will conflict with bug 838146, for what it's worth.  I can probably merge as needed, but just FYI.

Note that with the webidl-navigator patch mozMobileMessage and mozSms are in fact conditioned on MOZ_WEBSMS_BACKEND.

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 9 • 11 years ago

(In reply to Boris Zbarsky (:bz) from comment #8)
> The navigator parts of this look like they will conflict with bug 838146,
> for what it's worth.  I can probably merge as needed, but just FYI.

Thank you.

> Note that with the webidl-navigator patch mozMobileMessage and mozSms are in
> fact conditioned on MOZ_WEBSMS_BACKEND.

Well, I mean I wish no a single line of WebSMS code is ever *COMPILED* if there is no WebSMS on that platform.  I've just had a quick scan of your patches, and you basically moved a few lines around, not really make WebSMS "conditioned on MOZ_WEBSMS_BACKEND" for me.

	Boris Zbarsky [:bzbarsky]
	Comment 10 • 11 years ago

> Well, I mean I wish no a single line of WebSMS code is ever *COMPILED*

Sure.  That's not what this bug asked for initially, but I understand your goal.  The WebIDL patch makes those two properties invisible to page script ifndef MOZ_WEBSMS_BACKEND but the C++ code is still there, of course.

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 11 • 11 years ago

(In reply to Boris Zbarsky (:bz) from comment #10)
> Sure.  That's not what this bug asked for initially, but I understand your
> goal.  The WebIDL patch makes those two properties invisible to page script
> ifndef MOZ_WEBSMS_BACKEND but the C++ code is still there, of course.

Yes, we had a problem in last Madrid Work Week.  WebSMS is currently disabled on Fennec build, but somehow Fennec test cases were still included in B2G Birch CI and failed to run.  I really want to do something to prevent such things from happening again. :(

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 12 • 11 years ago

https://tbpl.mozilla.org/?tree=Try&rev=f8c20772c188

	Mounir Lamouri (:mounir)
	Comment 13 • 11 years ago

Comment on attachment 772653 [details] [diff] [review]
patch

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

The Mozilla project is heading in the exact opposite direction: we want to reduce the number of compile-time options because those options are a pain to maintain. When we added the MOZ_WEBSMS_BACKEND compile option, that was mostly to prevent the permission to show up at install time on Android, it was never in order to reduce the LOC being compiled. Usually, Gecko will not expose features that are disabled but will still have them compiled and we should keep that as is IMO.

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 14 • 11 years ago

According to Mounir's comment 13, marked as duplicate of bug 838146 instead.

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 15 • 11 years ago

Shall I re-open this bug?

	Masatoshi Kimura [:emk]
	Comment 16 • 11 years ago

Let's do this in bug 914182.

	Masatoshi Kimura [:emk]
	Comment 17 • 11 years ago

Ah, are you asking about SmsMessage/SmsEvent/MmsMessage/etc. ? I think it would be better to file a new bug.