Closed
Bug 1072535
Opened 10 years ago
Closed 10 years ago
[Gallery] Downloaded images will not appear in Gallery until Gallery is refreshed.
Categories
(Firefox OS Graveyard :: Gaia::Gallery, defect)
Tracking
(blocking-b2g:2.0+, firefox34 wontfix, firefox35 wontfix, firefox36 fixed, b2g-v1.4 unaffected, b2g-v2.0 verified, b2g-v2.0M verified, b2g-v2.1 verified, b2g-v2.2 verified)
People
(Reporter: jthomas, Assigned: dhylands)
References
()
Details
(Whiteboard: [2.1-exploratory-2])
Attachments
(9 files, 4 obsolete files)
155.72 KB,
text/plain
|
Details | |
46 bytes,
text/x-github-pull-request
|
dhylands
:
review+
|
Details | Review |
8.85 KB,
patch
|
mikeh
:
review+
|
Details | Diff | Splinter Review |
5.69 KB,
patch
|
Paolo
:
review+
|
Details | Diff | Splinter Review |
5.96 KB,
patch
|
dhylands
:
approval-mozilla-b2g34+
|
Details | Diff | Splinter Review |
15.50 KB,
patch
|
dhylands
:
approval-mozilla-b2g34+
|
Details | Diff | Splinter Review |
5.95 KB,
patch
|
bajaj
:
approval-mozilla-b2g32+
|
Details | Diff | Splinter Review |
15.48 KB,
patch
|
bajaj
:
approval-mozilla-b2g32+
|
Details | Diff | Splinter Review |
3.49 MB,
video/mp4
|
Details |
Description: If gallery has been previously opened and the user decides to download an image, the image will not appear in the gallery unless the gallery has been completely closed. Repro Steps: 1) Update a Flame to 20140924000243 2) Open Gallery 3) Download any image from the Browser. 4) Navigate back to Gallery. Actual: Newly downloaded images do not appear in Gallery. Expected: It is expected that the new images downloaded will appear in the Gallery. Environmental Variables: Device: Flame 2.1 KitKat Base (319mb) Build ID: 20140924000243 Gaia: 93a99bea0b40d81bd063f7d8b1964dc1ba35ba7b Gecko: d7e31ecd48af Version: 34.0a2 Firmware Version: L1TC10011800 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0 Repro frequency: 100% See attached: Logcat Video: https://www.youtube.com/watch?v=v0gl2qDaQ7w&edit=vd
Reporter | ||
Comment 1•10 years ago
|
||
This issue also occurs on the Flame 2.2 KitKat Base (319mb), Flame 2.1 KitKat Base (512mb) and the Flame 2.0 KitKat Base (319mb). Description: Newly downloaded images do not appear in Gallery when Gallery is already running in background. Flame 2.2 KitKat Base (319mb) Environmental Variables: Device: Flame Master Build ID: 20140924013003 Gaia: ff6dbb006e4e60edba697639aa2a2a199b07069b Gecko: 1e2993c99323 Version: 35.0a1 (Master) Firmware Version: v180 User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0 Flame 2.1 KitKat Base (512mb) Environmental Variables: Device: Flame 2.1 BuildID: 20140924000243 Gaia: 93a99bea0b40d81bd063f7d8b1964dc1ba35ba7b Gecko: d7e31ecd48af Version: 34.0a2 (2.1) Firmware: V180 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0 Flame 2.0 KitKat Base (319mb) Environmental Variables: Device: Flame 2.0 Build ID: 20140924003005 Gaia: 263e3b201dca967ec5346e35901aa981ca47dce7 Gecko: 35d791e16d31 Version: 32.0 (2.0) Firmware Version: v180 User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
QA Whiteboard: [QAnalyst-Triage?]
status-b2g-v2.0:
--- → affected
status-b2g-v2.1:
--- → affected
status-b2g-v2.2:
--- → affected
Flags: needinfo?(dharris)
Whiteboard: [2.1-exploratory-2]
adding qawanted to recheck branches on a shallow flash to see if the issue still occurs
Flags: needinfo?(dharris)
Keywords: qawanted
Updated•10 years ago
|
Flags: needinfo?(dharris)
Comment 3•10 years ago
|
||
Tested with Shallow Flash on 319mb This bug repro's on Flame KK builds: Flame 2.2 KK, Flame 2.1 KK, Flame 2.0 KK, Flame 2.0 Base KK Actual Results: With Gallery already open, images that are downloaded won't show up in the Gallery until the gallery is refreshed. Repro Rate: 4/4 Environmental Variables: Device: Flame Master KK BuildID: 20141001060621 Gaia: a23d2c490b39c4699c9375e25c4acdf396a2fa85 Gecko: 835ef55e175e Version: 35.0a1 (Master) Firmware Version: L1TC10011800 User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0 ----------------------------------------------------------------- Environmental Variables: Device: Flame 2.1 KK BuildID: 20141001060122 Gaia: b327c640fea887770d011a127e349838b3b44724 Gecko: 7359d0d0222d Version: 34.0a2 Firmware Version: L1TC10011800 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0 ----------------------------------------------------------------- Environmental Variables: Device: Flame 2.0 BuildID: 20141001060124 Gaia: 8079cba2133e6f5443dba24dad077f7f91e6c978 Gecko: 66c1ea78b6c1 Version: 32.0 (2.0) Firmware Version: L1TC10011800 User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0 ----------------------------------------------------------------- Environmental Variables: Device: Flame 2.0 BuildID: 20140904160718 Gaia: 506da297098326c671523707caae6eaba7e718da Gecko: Gonk: Version: 32.0 (2.0) Firmware: V180 User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
Updated•10 years ago
|
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell)
Comment 4•10 years ago
|
||
[Blocking Requested - why for this release]: This is not a regression but it feels like users would expect the gallery to update 'live' and NOT have to stop the app and restart it.
blocking-b2g: --- → 2.0?
Comment 5•10 years ago
|
||
Seems like a dup of https://bugzilla.mozilla.org/show_bug.cgi?id=1074632
Comment 6•10 years ago
|
||
Hema - this issue was written first AND has more work complete (branch checks) - don't you think the other bug should be closed as a dupe (just asking)?
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → dhylands
blocking-b2g: 2.0? → 2.0+
Updated•10 years ago
|
Flags: needinfo?(dharris)
Updated•10 years ago
|
status-b2g-v1.4:
--- → unaffected
Assignee | ||
Comment 10•10 years ago
|
||
I submitted a try run to see if the updated test passes: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=5d8bee6f59e8 I'll do something a bit more extensive once I see the linux ones passing.
Attachment #8501369 -
Flags: review?(aus)
Assignee | ||
Comment 11•10 years ago
|
||
Assignee | ||
Comment 12•10 years ago
|
||
Attachment #8501379 -
Flags: review+
Assignee | ||
Comment 13•10 years ago
|
||
Attachment #8501370 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8501387 -
Flags: review?(mhabicher)
Assignee | ||
Comment 14•10 years ago
|
||
I needed to change the event data sent from created to modified to be consistent with the other media file producers. And I tested in gallery this time.
Attachment #8501369 -
Attachment is obsolete: true
Attachment #8501369 -
Flags: review?(aus)
Attachment #8501407 -
Flags: review?(aus)
Comment 15•10 years ago
|
||
Comment on attachment 8501407 [details] [diff] [review] Pt 1 - Have Download API notify device storage when a download completes. v2 Review of attachment 8501407 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/downloads/DownloadsAPI.jsm @@ +143,5 @@ > + > + // Add test to verify that the download-watcher-notify event was actually sent. > + // Put it in test_downloads_basic.html > + > + if (download.state == "succeeded") { A download succeeding is not guaranteed to be the last notification you receive for that download. It is also possible that a successful download is added to the list without transitioning from other states. As I understand it, you need to fire a "download-watcher-notify" observer notification because you need to interface with the Device Storage code, which is C++ only, and an observer notification is simpler than calling a method from JavaScript on the C++ interfaces. However, this operation is required in B2G only, because other platforms don't use Device Storage, so it doesn't belong to the Downloads system for all platforms. I'd use a JS component tied to Device Storage, listening to the same onDownloadAdded / onDownloadChanged notifications from the JavaScript API for Downloads, similar to what you see here, but keeping track of which downloads have already been completed (on the assumption that notifying multiple times has a performance issue). As you've noticed, the DOM Downloads API sends notifications to child processes using nsIMessageBroadcaster, and as I understand it you're working entirely in the parent process, so apparently you shouldn't be a client of "Downloads:Added" and "Downloads:Changed" messages, despite the concept would be the same for those notifications. Hope this makes sense! :-)
Attachment #8501407 -
Flags: review?(aus)
Assignee | ||
Comment 16•10 years ago
|
||
(In reply to :Paolo Amadini from comment #15) > Comment on attachment 8501407 [details] [diff] [review] > Pt 1 - Have Download API notify device storage when a download completes. v2 > > Review of attachment 8501407 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: dom/downloads/DownloadsAPI.jsm > @@ +143,5 @@ > > + > > + // Add test to verify that the download-watcher-notify event was actually sent. > > + // Put it in test_downloads_basic.html > > + > > + if (download.state == "succeeded") { > > A download succeeding is not guaranteed to be the last notification you > receive for that download. It is also possible that a successful download is > added to the list without transitioning from other states. Would that mean that a new file was created? > As I understand it, you need to fire a "download-watcher-notify" observer > notification because you need to interface with the Device Storage code, > which is C++ only, and an observer notification is simpler than calling a > method from JavaScript on the C++ interfaces. However, this operation is > required in B2G only, because other platforms don't use Device Storage, so > it doesn't belong to the Downloads system for all platforms. I believe that DeviceStorage is available on all platforms. > I'd use a JS component tied to Device Storage, listening to the same > onDownloadAdded / onDownloadChanged notifications from the JavaScript API > for Downloads, similar to what you see here, but keeping track of which > downloads have already been completed (on the assumption that notifying > multiple times has a performance issue). Currently DeviceStorage has no JS component, so this is the part where I'm lost. How do you get the navigator.mozDownloadManager? The code I'd write wouldn't have a window, so my understanding is that the navigator doesn't exist. > As you've noticed, the DOM Downloads API sends notifications to child > processes using nsIMessageBroadcaster, and as I understand it you're working > entirely in the parent process, so apparently you shouldn't be a client of > "Downloads:Added" and "Downloads:Changed" messages, despite the concept > would be the same for those notifications. > > Hope this makes sense! :-) At a conceptual level, it makes sense, but I'm not sure where to start.
Assignee | ||
Comment 17•10 years ago
|
||
More complete try run: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=f2d48b638f89 Does anybody know where the download tests are run? I looked through M1 thru M5 and can't find any evidence (I was searching for test_down)
Assignee | ||
Comment 18•10 years ago
|
||
Now I'm confused. Paolo, is DownloadAPI B2G only? comment 15 seems to imply that is isn't But the tests are only run for B2G: http://dxr.mozilla.org/mozilla-central/source/dom/downloads/moz.build#7-8
Flags: needinfo?(paolo.mozmail)
Comment 19•10 years ago
|
||
Comment on attachment 8501387 [details] [diff] [review] Pt 2 - Have devicestorage notify all storage types based on extension. v2 Review of attachment 8501387 [details] [diff] [review]: ----------------------------------------------------------------- This looks good to me, with the nits/questions below addressed. ::: dom/devicestorage/nsDeviceStorage.cpp @@ +459,5 @@ > +{ > + // This function determines if aType shares a root directory with the > + // other media types (so only applies to music, videos, pictures and sdcard). > + > + if (aType.EqualsLiteral(DEVICESTORAGE_APPS) || Would it be better/more future-proof to explicitly test for DEVICESTORAGE_PICTURES, _VIDEOS, _MUSIC, and _SDCARD here? @@ +466,5 @@ > + } > +#ifdef MOZ_WIDGET_GONK > + return true; > +#else > + // For dekstop, if the directories have been overridden, then they share nit: desktop @@ +495,5 @@ > > NS_IMETHODIMP > FileUpdateDispatcher::Observe(nsISupports *aSubject, > const char *aTopic, > const char16_t *aData) nit: not in your patch, but the *'s above should go with the type. @@ +544,5 @@ > +#endif > + dsf = new DeviceStorageFile(NS_LITERAL_STRING(DEVICESTORAGE_SDCARD), volName, path); > + > + } else if (!strcmp(aTopic, "file-watcher-notify")) { > + dsf = static_cast<DeviceStorageFile*>(aSubject); do_QueryInterface()? @@ +596,2 @@ > } else { > + obs->NotifyObservers(dsf, "file-watcher-update", aData); Can we slurp out references to "file-watcher-update" to a const char* kFileWatcherUpdateTopic?
Attachment #8501387 -
Flags: review?(mhabicher) → review+
Comment 20•10 years ago
|
||
(In reply to Dave Hylands [:dhylands] from comment #18) > Now I'm confused. > > Paolo, is DownloadAPI B2G only? > comment 15 seems to imply that is isn't > But the tests are only run for B2G: > http://dxr.mozilla.org/mozilla-central/source/dom/downloads/moz.build#7-8 It's a little confusing for sure, we have a few download apis :) The jsdownloads component is included in firefox desktop, android and b2g. It lives here -- http://mxr.mozilla.org/mozilla-central/source/toolkit/components/jsdownloads/ The Downloads *Web API* is currently *only* on b2g builds that are *NOT* for desktop. It lives here -- http://mxr.mozilla.org/mozilla-central/source/dom/downloads/ Hope that helps. Some of the Download Web API tests are disabled on b2g emulator because of intermittent issues but should be re-enabled in the near future.
Flags: needinfo?(paolo.mozmail)
Assignee | ||
Comment 21•10 years ago
|
||
I figured out how to do what needs to be done purely in C++. The test fails, so I can't land it as is. I need to figure out how to enable SpecialPowers. This is the try run and failure: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=3796b753fa13 Manual testing works though, so I'd also be willing to fix the test up in a folloup bug.
Attachment #8501407 -
Attachment is obsolete: true
Attachment #8502260 -
Flags: review?(paolo.mozmail)
Comment 22•10 years ago
|
||
(In reply to Ghislain Aus Lacroix [:aus] from comment #20) > The Downloads *Web API* is currently *only* on b2g builds that are *NOT* for > desktop. It lives here -- > http://mxr.mozilla.org/mozilla-central/source/dom/downloads/ > > Hope that helps. Some of the Download Web API tests are disabled on b2g > emulator because of intermittent issues but should be re-enabled in the near > future. Ah, that helps, thanks!
Comment 23•10 years ago
|
||
Comment on attachment 8502260 [details] [diff] [review] Pt 1 - Have Download API notify device storage when a download completes. v3 The approach of using a notification for Device Storage in the platform-specific section of the code looks good to me. This doesn't need to be C++, however, we should work on the JavaScript side when possible. The place you have to modify is here: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/jsdownloads/src/DownloadIntegration.jsm#615 You'd need a similar try-catch, and you can use Services.prefs.getBoolPref and Services.obs to access the services you need. (In reply to Dave Hylands [:dhylands] from comment #21) > The test fails, so I can't land it as is. I need to figure out how to enable > SpecialPowers. These are xpcshell tests, so you should use Services.obs and Services.prefs directly. You'll need to register a cleanup function, similarly to what is done here: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/jsdownloads/test/unit/head.js#709 Note that Services.prefs.clearUserPref doesn't always yield the expected result (depending on how the profile is set up), you may need to call setBoolPref with the old value instead. For simplicity, I suggest you add a check to the existing platform integration test, rather than adding a new one. The test you need to modify is here: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/jsdownloads/test/unit/common_test_Download.js#1728
Attachment #8502260 -
Flags: review?(paolo.mozmail) → feedback+
Assignee | ||
Comment 24•10 years ago
|
||
I redid the test as you suggested. Here's my successful try run: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=1f86d4c9b5d3 Currently, I don't see the advantage of changing the NotifyObservers code from C++ to JS, so I just left it as C++. Won't writing it in JS wind up using more memory?
Attachment #8502260 -
Attachment is obsolete: true
Attachment #8502926 -
Flags: review?(paolo.mozmail)
Comment 25•10 years ago
|
||
Comment on attachment 8502926 [details] [diff] [review] Pt 1 - Have Download API notify device storage when a download completes. v4 Review of attachment 8502926 [details] [diff] [review]: ----------------------------------------------------------------- (In reply to Dave Hylands [:dhylands] from comment #24) > Currently, I don't see the advantage of changing the NotifyObservers code > from C++ to JS, so I just left it as C++. Won't writing it in JS wind up > using more memory? Our final goal for the "jsdownloads" module is to remove all the C++ code, so that the module is implemented using only one language. We still have C++ code, partly because in some cases there is no JavaScript alternative, and partly because we didn't have a strong need to migrate to js-ctypes yet. This is why I'd prefer for this to be done in JavaScript, though it's not a big deal if you prefer to keep this C++, this bit would be the easiest to migrate later. The advantage of JavaScript here would be conciseness. The C++ code needs more lines of code, in fact this implementation has some issues I didn't point out earlier, that need to be fixed: ::: toolkit/components/jsdownloads/src/DownloadPlatform.cpp @@ +126,5 @@ > ::CFRelease(observedObject); > #endif > + if (mozilla::Preferences::GetBool("device.storage.enabled", true)) { > + // Tell DeviceStorage that a new file may have been added. > + nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService(); This needs to be null-checked. @@ +128,5 @@ > + if (mozilla::Preferences::GetBool("device.storage.enabled", true)) { > + // Tell DeviceStorage that a new file may have been added. > + nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService(); > + nsCOMPtr<nsISupportsString> pathString > + = do_CreateInstance(NS_SUPPORTS_STRING_CONTRACTID); This too, or check rv. @@ +129,5 @@ > + // Tell DeviceStorage that a new file may have been added. > + nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService(); > + nsCOMPtr<nsISupportsString> pathString > + = do_CreateInstance(NS_SUPPORTS_STRING_CONTRACTID); > + pathString->SetData(path); This is fallible as well, need to check rv. @@ +131,5 @@ > + nsCOMPtr<nsISupportsString> pathString > + = do_CreateInstance(NS_SUPPORTS_STRING_CONTRACTID); > + pathString->SetData(path); > + obs->NotifyObservers(pathString, "download-watcher-notify", > + MOZ_UTF16("modified")); nit: (void)obs->NotifyObservers to make it explicit that we don't care about whether this is successful. With regard to JavaScript memory usage, I don't think there's any difference, but it's a secondary concern anyways given the frequency with which this code is called.
Attachment #8502926 -
Flags: review?(paolo.mozmail) → review+
Assignee | ||
Comment 26•10 years ago
|
||
I filed followup bug 1082714 to rewrite the devices storage notification in JS.
Assignee | ||
Comment 27•10 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/902dfed4c120 https://hg.mozilla.org/integration/b2g-inbound/rev/9d0bba1b36dd
Assignee | ||
Comment 28•10 years ago
|
||
And my merge to gaia https://github.com/mozilla-b2g/gaia/commit/e7eec5a773865f8b6557d05927ac902782eaa41c NOTE: If for some reason this bug needs to be backed out, only the gecko portion needs to be backed out. The gaia portion only affects my test app, and the test app doesn't need the gecko portion.
Comment 29•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/902dfed4c120 https://hg.mozilla.org/mozilla-central/rev/9d0bba1b36dd
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S7 (24Oct)
Comment 31•10 years ago
|
||
Issue is verified fixed in Flame 2.2 Actual Results: Newly saved images downloaded via Browser appear in Gallery app correctly. Device: Flame Master Build ID: 20141016040204 Gaia: 841d0d7d1b879f0ff4b5a8727f5dd23c7b0000a9 Gecko: a280a03c9f3c Version: 36.0a1 (Master) Firmware Version: v180 User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0 ------------------------------------------------------------------------------- ------------------------------------------------------------------------------- Waiting for patch uplift for Flame branches: 2.1, 2.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Assignee | ||
Comment 32•10 years ago
|
||
Comment on attachment 8501387 [details] [diff] [review] Pt 2 - Have devicestorage notify all storage types based on extension. v2 NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] Bug caused by (feature/regressing bug #): MTP User impact if declined: Gallery/Music/Video app don't update to reflect deleted files. Testing completed: TBPL, manual testing. Risk to taking this patch (and alternatives if risky): I think that the patch is fairly low risk. String or UUID changes made by this patch: None
Attachment #8501387 -
Flags: approval-mozilla-b2g34?
Attachment #8501387 -
Flags: approval-mozilla-b2g32?
Assignee | ||
Comment 33•10 years ago
|
||
Comment on attachment 8502926 [details] [diff] [review] Pt 1 - Have Download API notify device storage when a download completes. v4 NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] Bug caused by (feature/regressing bug #): MTP User impact if declined: Gallery/Music/Video app don't update to reflect deleted files. Testing completed: TBPL, manual testing. Risk to taking this patch (and alternatives if risky): I think that the patch is fairly low risk. String or UUID changes made by this patch: None
Attachment #8502926 -
Flags: approval-mozilla-b2g34?
Attachment #8502926 -
Flags: approval-mozilla-b2g32?
Updated•10 years ago
|
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Updated•10 years ago
|
Attachment #8501387 -
Flags: approval-mozilla-b2g34? → approval-mozilla-b2g34+
Updated•10 years ago
|
Attachment #8502926 -
Flags: approval-mozilla-b2g34? → approval-mozilla-b2g34+
Comment 34•10 years ago
|
||
Needs rebasing for uplift.
Flags: needinfo?(dhylands)
Keywords: branch-patch-needed
Assignee | ||
Comment 35•10 years ago
|
||
[Triage Comment] Transferred approval-b2g34+ from the other patch. This patch has been rebased on b2g34.
Flags: needinfo?(dhylands)
Attachment #8507213 -
Flags: approval-mozilla-b2g34+
Assignee | ||
Comment 36•10 years ago
|
||
[Triage Comment] Transferred approval-b2g34+ from the other patch. This patch has been rebased on b2g34.
Attachment #8507214 -
Flags: approval-mozilla-b2g34+
Assignee | ||
Comment 37•10 years ago
|
||
Comment on attachment 8502926 [details] [diff] [review] Pt 1 - Have Download API notify device storage when a download completes. v4 Cleared b2g34+ since this patch doesn't apply cleanly.
Attachment #8502926 -
Flags: approval-mozilla-b2g34+
Assignee | ||
Comment 38•10 years ago
|
||
Comment on attachment 8501387 [details] [diff] [review] Pt 2 - Have devicestorage notify all storage types based on extension. v2 Cleared b2g34+ since this patch doesn't apply cleanly.
Attachment #8501387 -
Flags: approval-mozilla-b2g34+
Assignee | ||
Comment 39•10 years ago
|
||
NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] Bug caused by (feature/regressing bug #): MTP User impact if declined: Gallery/Music/Video app don't update to reflect deleted files. Testing completed: TBPL, manual testing. Risk to taking this patch (and alternatives if risky): I think that the patch is fairly low risk. String or UUID changes made by this patch: None
Attachment #8507248 -
Flags: approval-mozilla-b2g32?
Assignee | ||
Comment 40•10 years ago
|
||
NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] Bug caused by (feature/regressing bug #): MTP User impact if declined: Gallery/Music/Video app don't update to reflect deleted files. Testing completed: TBPL, manual testing. Risk to taking this patch (and alternatives if risky): I think that the patch is fairly low risk. String or UUID changes made by this patch: None
Attachment #8507249 -
Flags: approval-mozilla-b2g32?
Assignee | ||
Comment 41•10 years ago
|
||
Comment on attachment 8501387 [details] [diff] [review] Pt 2 - Have devicestorage notify all storage types based on extension. v2 Removed b2g32? from this patch since I attached a b2g32 specific patch.
Attachment #8501387 -
Flags: approval-mozilla-b2g32?
Assignee | ||
Comment 42•10 years ago
|
||
Comment on attachment 8502926 [details] [diff] [review] Pt 1 - Have Download API notify device storage when a download completes. v4 Removed b2g32? from this patch since I attached a b2g32 specific patch.
Attachment #8502926 -
Flags: approval-mozilla-b2g32?
Comment 43•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/f938d9dc837c https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/105d1bb4180f
status-firefox34:
--- → wontfix
status-firefox35:
--- → wontfix
status-firefox36:
--- → fixed
Keywords: branch-patch-needed
Updated•10 years ago
|
Attachment #8507248 -
Flags: approval-mozilla-b2g32? → approval-mozilla-b2g32+
Updated•10 years ago
|
Attachment #8507249 -
Flags: approval-mozilla-b2g32? → approval-mozilla-b2g32+
Comment 44•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/b80917f7a74e https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/76f8ae79cd8e
Comment 45•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/b80917f7a74e https://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/76f8ae79cd8e
status-b2g-v2.0M:
--- → fixed
Comment 46•10 years ago
|
||
This issue is verified fixed on Flame 2.0 and 2.1. Result: The downloaded image is displayed in Gallery app properly without closing and re-opening the app. Device: Flame 2.0 (319mb, KK, Full Flash) BuildID: 20141105160207 Gaia: 5ee26701a4d8db266bfb203b2179f686ce14d8b6 Gecko: dbf49343e889 Gonk: 48835395daa6a49b281db62c50805bd6ca24077e Version: 32.0 (2.0) Firmware Version: v188 User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0 Device: Flame 2.1 (319mb, KK, Full Flash) BuildID: 20141106001204 Gaia: 9658b93b412bdcc0f953d668e8c8e68318c99fb8 Gecko: 76880403db44 Gonk: 48835395daa6a49b281db62c50805bd6ca24077e Version: 34.0 (2.1) Firmware Version: v188 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
Updated•10 years ago
|
QA Contact: croesch
Updated•10 years ago
|
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Comment 47•10 years ago
|
||
Verify passed, this issue can't be repro on Woodduck 2.0. Attached: Verify_Woodduck_Figuredisplay.mp4 Reproducing rate: 0/5 Gaia-Rev ee5cf148b4c546beea9bfb799d2a3ee62074957d Gecko-Rev 73601b71861cbc2f180c4d2653cec3e9fbb39db5 Build-ID 20141114050313 Version 32.0
Comment 48•10 years ago
|
||
Comment 49•9 years ago
|
||
Comment on attachment 8501387 [details] [diff] [review] Pt 2 - Have devicestorage notify all storage types based on extension. v2 >+ static const nsLiteralString kMediaTypes[] = { nsLiteralString isn't a POD type, so creating static instances might not be ideal. (But I haven't looked at what compilers actually do here, so you might be OK.)
Assignee | ||
Comment 50•9 years ago
|
||
(In reply to neil@parkwaycc.co.uk from comment #49) > Comment on attachment 8501387 [details] [diff] [review] > Pt 2 - Have devicestorage notify all storage types based on extension. v2 > > >+ static const nsLiteralString kMediaTypes[] = { > nsLiteralString isn't a POD type, so creating static instances might not be > ideal. (But I haven't looked at what compilers actually do here, so you > might be OK.) Good point. I've filed bug 1157062 to rectify this.
You need to log in
before you can comment on or make changes to this bug.
Description
•