1117663 - [FFOS7715 v2.1][STK]USAT case 27.22.4.1.5 DISPLAY TEXT SEQ 5.1 display of basic icon fail

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[jinghua.xing]

[Testing Steps     ]:USAT case
27.22.4.1.5 DISPLAY TEXT SEQ 5.1A  DISPLAY TEXT, display of basic icon, self-explanatory, successful
27.22.4.1.5 DISPLAY TEXT SEQ 5.1B  DISPLAY TEXT, display of basic icon, self-explanatory, requested icon could not be displayed

[Expected Result  ]:case pass

[Test Result      ]:
27.22.4.1.5 DISPLAY TEXT SEQ 5.1A  only show text BASIC-ICON，TR returned successsfully ，but haven't showed BASIC-ICON
27.22.4.1.5 DISPLAY TEXT SEQ 5.1B  show text BASIC-ICON，TR return 0x00 wrong，expected return 0x04（General Result: Command performed successfully but requested icon could not be displayed）

Comment 1

[jinghua.xing]

As we can't display the icon, so we should made the TR 0x04 but not 0x00

Comment 2

[jinghua.xing]

We operator this case as B case

Comment 3

[jinghua.xing]

Attachment: stk_display_icon.patch

gaia patch for this issue

Dear Shawn,

I have made a patch for sending right response for this issue, please help review and check if it is the fine way to fix the issue.

Thanks!

Comment 4

[jinghua.xing]

Attachment: stk_display_gecko.patch

gecko patch for this issue.

Comment 5

[shawn ku [:sku]]

Comment on attachment 8544969 [details] [diff] [review]
stk_display_gecko.patch

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

Hi Edgar,
 Could you please give some comment here to partner?


By checking 11.14, clause 12.12, I will give this as okay case.
however, it was because gaia did not finish icon display part yet.

Anyway, we should make result code up @ MozIccManager.webidl

Comment 6

[shawn ku [:sku]]

Comment on attachment 8544968 [details] [diff] [review]
stk_display_icon.patch

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

Hi Fernando:
 Icon display on 2.1 is not ready yet, I will treat this patch as 2.1s only fix.

Could you please help check/review, and provide some valuable suggestion here?
Besides, any test case needed?

Comment 7

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

Comment on attachment 8544968 [details] [diff] [review]
stk_display_icon.patch

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

LGTM

Comment 8

[Edgar Chen [:edgar]]

Comment on attachment 8544969 [details] [diff] [review]
stk_display_gecko.patch

Looks good. But this is not a formal patch, so I give f+ instead. Thank you.

(In reply to shawn ku [:sku] from comment #5)
> Comment on attachment 8544969 [details] [diff] [review]
> stk_display_gecko.patch
> 
> Review of attachment 8544969 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Hi Edgar,
>  Could you please give some comment here to partner?
> 
> By checking 11.14, clause 12.12, I will give this as okay case.

And in 11.14, clause 6.5.4, 
--
If the SIM provides an icon identifier with a proactive command, then the ME shall inform the SIM if the icon could not be displayed by sending the general result "Command
performed successfully, but requested icon could not be displayed"
--

Comment 9

[Steven Yang [:styang]]

As triage, plus it

Comment 10

[Steven Yang [:styang]]

Edgar, as you're working on this, assign it to you.

Comment 11

[Noemí Freire (:noemi)]

Just adding that Bug 1016807 will be landed soon in master and it offers STK icon display (gaia support) for all the proactive commands that may contain an icon.

Comment 12

[Steven Yang [:styang]]

Edgar, what's the progress of this issue?

Comment 13

[Edgar Chen [:edgar]]

(In reply to Steven Yang [:styang] from comment #12)
> Edgar, what's the progress of this issue?

Since this bug is filed for Gaia, I am going to file a separated bug for Gecko and will attach formal gecko patch there. Keeps ni? to me for tacking.

Comment 14

[Noemí Freire (:noemi)]

Hi Edgar,

We thought that gecko part was covered by Bug 824145 (already landed), is still something missing on gecko side?. Thanks!

Comment 15

[Edgar Chen [:edgar]]

(In reply to Noemí Freire (:noemi) from comment #14)
> Hi Edgar,
> 
> We thought that gecko part was covered by Bug 824145 (already landed), is
> still something missing on gecko side?. Thanks!

Hi noemi, the supporting of stk icon in gecko was finished in bug 824145, correct. What we miss is just a const of result code defined in WebIDL [1] for the case that app is unable to display the icon for some reason. In such case, app should send the TR with special result code, 0x04, which means "Command performed successfully, but requested icon could not be displayed" [2].

Thank you.

[1] Please see attachment #8544969 [details] [diff] [review].
[2] Please see comment #8.

Comment 16

[Edgar Chen [:edgar]]

(In reply to Edgar Chen [:edgar][:echen] from comment #13)
> (In reply to Steven Yang [:styang] from comment #12)
> > Edgar, what's the progress of this issue?
> 
> Since this bug is filed for Gaia, I am going to file a separated bug for
> Gecko and will attach formal gecko patch there. Keeps ni? to me for tacking.

(In reply to Edgar Chen [:edgar][:echen] from comment #15)
> (In reply to Noemí Freire (:noemi) from comment #14)
> > Hi Edgar,
> > 
> > We thought that gecko part was covered by Bug 824145 (already landed), is
> > still something missing on gecko side?. Thanks!
> 
> Hi noemi, the supporting of stk icon in gecko was finished in bug 824145,
> correct. What we miss is just a const of result code defined in WebIDL [1]
> for the case that app is unable to display the icon for some reason. In such
> case, app should send the TR with special result code, 0x04, which means
> "Command performed successfully, but requested icon could not be displayed"
> [2].
> 
> Thank you.
> 
> [1] Please see attachment #8544969 [details] [diff] [review] [diff] [review].
> [2] Please see comment #8.

Bug 1129946 is filed for gecko part.

Comment 17

[Sean Lee [:seanlee][:weilonge]]

Because gecko patch (bug 1129946) is not landed to v2.1s yet, I will land the Gaia patch later.

Comment 18

[GitHub Autolander Bot]

Attachment: [gaia] weilonge:seanlee/STK/v2.1s/Bug1117663 > mozilla-b2g:v2.1s

Comment 19

[Sean Lee [:seanlee][:weilonge]]

v2.1s: https://github.com/mozilla-b2g/gaia/commit/485b69e877429b85d08129ddd1cc2c22fd4806ce

Comment 20

[jinghua.xing]

Sean:

Because also for STK_CMD_LAUNCH_BROWSER、STK_CMD_PLAY_TONE、STK_CMD_DISPLAY_TEXT、STK_CMD_GET_INKEY、STK_CMD_GET_INPUT、STK_CMD_SET_UP_IDLE_MODE_TEXT、STK_CMD_SET_UP_MENU and STK_CMD_SELECT_ITEM commands， the TR STK_RESULT_PRFRMD_ICON_NOT_DISPLAYED should be sent if there are icons in the command but not only for the STK_CMD_DISPLAY_TEXT command as the test cases in 3GPP TS 51.010-4 V12.1.0 says. So I think we should cover all the cases contains the icons to display. How do you think about this? Thank you.

Comment 21

[jinghua.xing]

The ETSI TS 101 267 6.4 also says the commands which have icons to display.

Comment 22

[Sean Lee [:seanlee][:weilonge]]

Hi Jinghua,

Could you help to fire a new bug to fix these items you mentioned at comment 20?
Let's discuss it at the new one.
Thank you.

Comment 23

[jinghua.xing]

I have fire a new Bug 1140971 for further discussion

Comment 24

[jinghua.xing]

Sean

Can you revert the patch you merged on the v2.1s and fix the bug together with Bug 1140971? Thank you.

Comment 25

[Sean Lee [:seanlee][:weilonge]]

Hi Jinghua,

Can you explain why we have to revert the patch? Is the patch something wrong?
Thank you.

Comment 26

[jinghua.xing]

Sean:

This patch is right but have not cover all the cases need to send 0x04 back for the stk command. So I think we can define a method in icc_worker and use the method to send TR back for the commands which may have icons to display. What do you think?

Thank you.

Comment 27

[Sean Lee [:seanlee][:weilonge]]

Hi Jinghua,

Sorry that I think we should remain this patch because it won't bring us any regression or other defects.
This patch seems good but we can provide a better solution, so let's discuss the new design at bug 1140971.

Thank you.

Comment 28

[Sean Lee [:seanlee][:weilonge]]

Comment on attachment 8574495 [details] [review]
[gaia] weilonge:seanlee/STK/v2.1s/Bug1117663 > mozilla-b2g:v2.1s

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined:
We did not follow the spec:
27.22.4.1.5 DISPLAY TEXT SEQ 5.1B  show text BASIC-ICON，TR return 0x00 wrong，expected return 0x04（General Result: Command performed successfully but requested icon could not be displayed）

TR should be 0x04 instead of 0x00 when there is no icon support in STK. 

[Testing completed]:
Partner tested PASS.

[Risk to taking this patch] (and alternatives if risky):
Very minor because it is only added a condition to send 0x00 or 0x04.

[String changes made]:
None

Comment 29

[Ryan VanderMeulen [:RyanVM]]

I'm confused, the PR in this bug shows as merged a week ago. The blocking status says 2.1S+. But we want this on v2.1 now too?

Comment 30

[Sean Lee [:seanlee][:weilonge]]

Hi Ryan,

This PR was merged to v2.1s, but v2.1 is still affected.
I suppose v2.1 branch should have this patch too.
Please correct me if there is anything incorrect.
Thank you.

Comment 31

[Ryan VanderMeulen [:RyanVM]]

v2.1: https://github.com/mozilla-b2g/gaia/commit/13c85d57f49b4bfd657ff674f2b530c141c94803