885685 - B2G MMS: Use nsIURI Object in applyFilter for MmsProxyFilter.

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Some content location from some MMSC will contain port:80. But it will be ignore in prePath. That should be a problem.

Comment 1

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

This bug is found by LEO. Nominate to leo+.

Comment 2

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: Patch v1.0

Comment 3

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 765880 [details] [diff] [review]
Patch v1.0

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

::: dom/mobilemessage/src/ril/MmsService.js
@@ +379,5 @@
>    return conn;
>  });
>  
>  function MmsProxyFilter(url) {
> +  this.url = Services.io.newURI(url,null,null); ;

nit: SP after comma, and remove the extra semicolon in the end.

Comment 4

[Lukas Blakk [:lsblakk] use ?needinfo]

Unless this is a certification blocker or leo wants to champion blocking on this we'll just wait for a fix in the next version. Found by != blocker.

Comment 5

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: Patch v1.1

Comment 6

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Need a fix for bug 879675.

Comment 7

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: Patch v1.2

Comment 8

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Comment on attachment 767009 [details] [diff] [review]
Patch v1.2

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

For bug 879675.

Comment 9

[Leo]

Discussed with CTAI. This should be a leo+. It resolves the issue of a patch which removes /.

Comment 10

[Gene Lian (I already quit Mozilla)]

Comment on attachment 767009 [details] [diff] [review]
Patch v1.2

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

Looks good!

::: dom/mobilemessage/src/ril/MmsService.js
@@ +373,5 @@
>    return conn;
>  });
>  
>  function MmsProxyFilter(url) {
> +  this.url = Services.io.newURI(url,null,null);

Please add spaces among parameters.

@@ +384,5 @@
>  
>    applyFilter: function applyFilter(proxyService, uri, proxyInfo) {
> +    if (!this.url.equals(uri)) {
> +      if (DEBUG) debug("applyFilter: content uri = " + JSON.stringify(this.url) +
> +                       " is not matched url = " + JSON.stringify(uri) + " .");

s/is not matched/is not matched with/

Comment 12

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: Patch v1.3

Comment 13

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: Patch v1.4

Rename this.url to this.uri.

Comment 14

[Gene Lian (I already quit Mozilla)]

https://hg.mozilla.org/projects/birch/rev/428cd9da48d3
https://hg.mozilla.org/releases/mozilla-b2g18/rev/29933d1937db

Comment 15

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/428cd9da48d3

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/29933d1937db