Closed Bug 649381 Opened 12 years ago Closed 12 years ago

implement moz-icon android support

Categories

(Core Graveyard :: Widget: Android, defect)

Product:
Core Graveyard
Component:
Widget: Android
Platform:
ARM
Android
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox7 fixed, fennec7+)

Status:
VERIFIED FIXED
Milestone:
mozilla7
Tracking Flags:
Tracking Status
firefox7 --- fixed
fennec 7+ ---

People

(Reporter: blassey, Assigned: alexp)

References

Details

(Keywords: verified-aurora)

Attachments

(1 file, 3 obsolete files)

Fix v2
23.24 KB, patch
alexp
: review+
Details | Diff | Splinter Review 
      No description provided.
OS: Linux → Android
Hardware: x86_64 → ARM
Whiteboard: [fennec-7]
tracking-fennec: ? → 7+
Whiteboard: [fennec-7]
Attached patch [WIP] Stub decoder (obsolete) — — Splinter Review 
Just an empty decoder module to see if it's hooked up properly.
Attached patch [WIP] libxul-config.mk change (obsolete) — — Splinter Review 
One more bit, which actually enables the icon decoder component for Android.
Attached patch Fix (obsolete) — — Splinter Review 
The patch includes
- Java function to get the icon for a specific file extension
- AndroidBridge and IPC bits to call the Java code
- nsIconChannel decoder implementation for Android in libpr0n
- makefiles/config changes to use a proper theme and include the new decoder code in Android build
Attachment #537625 - Attachment is obsolete: true
Attachment #537898 - Attachment is obsolete: true
Attachment #539368 - Flags: review?(blassey.bugs)
Status: NEW → ASSIGNED
Comment on attachment 539368 [details] [diff] [review]
Fix

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

r=blassey assuming the theme change isn't required

::: configure.in
@@ -4850,5 @@
>          NSS_DISABLE_DBM=1
>          USE_ARM_KUSER=1
>          MOZ_INSTALLER=
>          NECKO_WIFI=
> -        MOZ_THEME_FASTSTRIPE=1

I don't think this change is needed, can you confirm and remove? or explain why its required.
Attachment #539368 - Flags: review?(blassey.bugs) → review+
(In reply to comment #6)
> ::: configure.in
...
> > -        MOZ_THEME_FASTSTRIPE=1
> 
> I don't think this change is needed, can you confirm and remove? or explain
> why its required.

I've double-checked that (actually triple-checked :)). It seems like it is not actually required. I tried different things, and was consistently getting the issues with this flag enabled. Apparently it was affected by something else. For some reason the changes in the module code and makefiles are not always picked up by the build, I had to remove the binary files manually to build properly.
Anyway, I'm submitting a patch without this change.
Attached patch Fix v2 — — Splinter Review 
Updated patch.
r=blassey
Attachment #539368 - Attachment is obsolete: true
Attachment #539961 - Flags: review+
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/12a9a11b2cbd
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
Verified Fixed

Nightly:
Mozilla/5.0 (Android; Linux armv7l; rv:8.0a1) Gecko/20110726 Firefox/8.0a1 Fennec/8.0a1

Aurora:
Mozila/5.0 (Android; Linux armv7l; rv:7.0a2) Gecko/20110726 Firefox/7.0a2 Fennec/7.0a2
Status: RESOLVED → VERIFIED
Keywords: verified-aurora
do we have tests for this?
Flags: in-testsuite?
Flags: in-litmus?(aaron.train)
Flags: in-litmus?(aaron.train)
Flags: in-testsuite?
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.