Closed Bug 1117663 Opened 9 years ago Closed 9 years ago

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

Categories

(Firefox OS Graveyard :: Gaia, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(blocking-b2g:2.1S+, b2g-v2.0 affected, b2g-v2.0M affected, b2g-v2.1 fixed, b2g-v2.1S fixed, b2g-v2.2 unaffected, b2g-master unaffected)

RESOLVED FIXED
2.2 S8 (20mar)
blocking-b2g 2.1S+
Tracking Status
b2g-v2.0 --- affected
b2g-v2.0M --- affected
b2g-v2.1 --- fixed
b2g-v2.1S --- fixed
b2g-v2.2 --- unaffected
b2g-master --- unaffected

People

(Reporter: Jinghua.Xing, Assigned: selee)

References

Details

(Whiteboard: [sprd390581])

Attachments

(2 files, 1 obsolete file)

[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)
As we can't display the icon, so we should made the TR 0x04 but not 0x00
Summary: [FFOS7715_v2.1]USAT case 27.22.4.1.5 DISPLAY TEXT SEQ 5.1 display of basic icon fail → [FFOS7715 v2.1][STK]USAT case 27.22.4.1.5 DISPLAY TEXT SEQ 5.1 display of basic icon fail
Whiteboard: [sprd390581]
We operator this case as B case
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!
Attachment #8544968 - Flags: review?(sku)
Attached patch stk_display_gecko.patch (obsolete) — Splinter Review
gecko patch for this issue.
Attachment #8544969 - Flags: review?(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
Attachment #8544969 - Flags: review?(sku) → review?(echen)
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?
Attachment #8544968 - Flags: review?(sku) → review?(frsela)
blocking-b2g: --- → 2.1S?
Comment on attachment 8544968 [details] [diff] [review]
stk_display_icon.patch

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

LGTM
Attachment #8544968 - Flags: review?(frsela) → review+
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"
--
Attachment #8544969 - Flags: review?(echen) → feedback+
Blocks: 1123554
As triage, plus it
blocking-b2g: 2.1S? → 2.1S+
Edgar, as you're working on this, assign it to you.
Assignee: nobody → echen
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.
No longer blocks: 1016807
See Also: → 1016807
Edgar, what's the progress of this issue?
Flags: needinfo?(echen)
(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.
Flags: needinfo?(echen)
Hi Edgar,

We thought that gecko part was covered by Bug 824145 (already landed), is still something missing on gecko side?. Thanks!
Flags: needinfo?(echen)
(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.
Blocks: 1129946
No longer blocks: 1129946
Depends on: 1129946
(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.
Assignee: echen → nobody
Flags: needinfo?(echen)
Attachment #8544969 - Attachment is obsolete: true
Because gecko patch (bug 1129946) is not landed to v2.1s yet, I will land the Gaia patch later.
Assignee: nobody → selee
Status: NEW → ASSIGNED
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.
Flags: needinfo?(selee)
The ETSI TS 101 267 6.4 also says the commands which have icons to display.
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.
Flags: needinfo?(selee)
I have fire a new Bug 1140971 for further discussion
Sean

Can you revert the patch you merged on the v2.1s and fix the bug together with Bug 1140971? Thank you.
Flags: needinfo?(selee)
Hi Jinghua,

Can you explain why we have to revert the patch? Is the patch something wrong?
Thank you.
Flags: needinfo?(selee)
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.
Flags: needinfo?(selee)
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.
Flags: needinfo?(selee)
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
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
Attachment #8574495 - Flags: approval-gaia-v2.1?
Attachment #8574495 - Flags: approval-gaia-v2.1? → approval-gaia-v2.1+
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?
Flags: needinfo?(selee)
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.
Flags: needinfo?(selee) → needinfo?(ryanvm)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: