1043169 - [Gallery]Delete screen is not proper for loading first time

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia::Gallery, defect)

Description

[vsireesha246]

Steps to reproduce:

open Gallery->Click on Camera->Take more than 50 pictures and videos->Click on Home button

Open->Gallery->Click on Select button(dotted box)->Select 2 to 3 images/video->Click on Delete icon->Observe first time(Small "cancel" and "Delete" buttons are shown first and then normal delete screen)(Here confirm.css is not applied properly)

Comment 1

[vsireesha246]

This bug might be regression of https://bugzilla.mozilla.org/show_bug.cgi?id=1011772

Comment 2

[vsireesha246]

Attachment: 2014-07-22-16-18-47.png

Comment 3

[Punam Dahiya [:pdahiya]]

Hi Sireesha

I am not able to replicate the issue on below environment

Device:Flame (RAM auto)
BuildId:20140723160203
OS Version: 2.1
Platform:34.0a1

Steps followed 
1. Open->Gallery (with ~200 images in it)
2. Click on Select button(dotted box)
3. Select 2 to 3 images
4. Click on Delete icon

I am able to see the delete confirmation dialog with confirm.css applied. It will help if you can provide more details such as buildid, device and logcat. Thanks

Comment 4

[vsireesha246]

Hi Punam,

Can you please follow the Steps mentioned below.

Precondition:
open Gallery->Click on Camera icon->Take more than 50 pictures or videos->Click on Home button

Open->Gallery->Click on Select button(dotted box)->Select 2 to 3 images/video->Click on Delete icon->Observe first time(Small "cancel" and "Delete" buttons are shown first and then normal delete screen)(Here confirm.css is not applied properly)

This issue just happen for a movement(very short time)that to first time loading only.Because confirm.css is added by dialog.js in runtime as Lazyload.load()

I tried uncommenting confirm.css in index.html and it is working fine.

Its happening with latest Master gaia.

Comment 5

[Russ Nicoletti [:russn]]

I am able to reproduce with Sireesha's STR. Her solution of uncommenting confirm.css in gallery index.html seems fine as video index.html includes confirm.css as well.

Comment 6

[Punam Dahiya [:pdahiya]]

(In reply to Russ Nicoletti [:russn] from comment #5)
> I am able to reproduce with Sireesha's STR. Her solution of uncommenting
> confirm.css in gallery index.html seems fine as video index.html includes
> confirm.css as well.

Hi Russ

I tried to replicate again this time on Hamachi (buildid 20140724160202) following below steps from #comment0 , but confirm delete dialog shows fine. 

open Gallery->Click on Camera->Take more than 50 pictures and videos->Click on Home button and 
Open->Gallery->Click on Select button(dotted box)->Select 2 to 3 images/video->Click on Delete icon

IMO, We should load confirm.css only when needed,in this case inside shared/js/dialogs.js.

In video app, we should comment confirm.css in index.html. However we are using confirm.css in showOverlay inside video.js. To not break overlays

1. One solution is to lazy load confirm.css in showOverlay inside video.js

2. Another approach, is to remove showOverlay method from video.js and use Dialogs.showOverlay(). IMO a better fix, wdyt?

Comment 7

[vsireesha246]

Hi Russ,

Would you please share logs,Build id and clear STR to Punam to reproduce this issue.(Because i cant able to share the logs)
She is not able to reproduce this issue.

Thanks..
Sireesha

(In reply to Russ Nicoletti [:russn] from comment #5)
> I am able to reproduce with Sireesha's STR. Her solution of uncommenting
> confirm.css in gallery index.html seems fine as video index.html includes
> confirm.css as well.

Comment 8

[Russ Nicoletti [:russn]]

Environment:

Gaia      3a06aa58245eaf848242d6d1497c1af536fffabd
Gecko     https://hg.mozilla.org/mozilla-central/rev/6c0971104909
BuildID   20140725040205
Version   34.0a1
Device    hamachi

To reproduce, I've found is that I need to do the following:

* When gallery app is coming up and thumbnails are being displayed, quickly tap on select button, then a photo, then delete.

For a brief moment, the small 'cancel' and 'delete' button appear at the top left of the screen. They appear for less time on hamachi than on flame. I've found the "small" buttons appear longer on flame.

I don't see any gallery info when I am running logcat.

Comment 9

[Russ Nicoletti [:russn]]

(In reply to Punam Dahiya from comment #6)
> (In reply to Russ Nicoletti [:russn] from comment #5)
> > I am able to reproduce with Sireesha's STR. Her solution of uncommenting
> > confirm.css in gallery index.html seems fine as video index.html includes
> > confirm.css as well.
> 
> Hi Russ
> 
> I tried to replicate again this time on Hamachi (buildid 20140724160202)
> following below steps from #comment0 , but confirm delete dialog shows fine. 
> 
> open Gallery->Click on Camera->Take more than 50 pictures and videos->Click
> on Home button and 
> Open->Gallery->Click on Select button(dotted box)->Select 2 to 3
> images/video->Click on Delete icon
> 
> IMO, We should load confirm.css only when needed,in this case inside
> shared/js/dialogs.js.
> 
> In video app, we should comment confirm.css in index.html. However we are
> using confirm.css in showOverlay inside video.js. To not break overlays
> 
> 1. One solution is to lazy load confirm.css in showOverlay inside video.js
> 
> 2. Another approach, is to remove showOverlay method from video.js and use
> Dialogs.showOverlay(). IMO a better fix, wdyt?

I like the idea of using Dialogs.showOverlay because that would remove what is basically duplicate code from video app. I will make that change on my box and test it out. 

However, it seems the issue with confirm.css will still exist. See comment #8.

Comment 10

[Punam Dahiya [:pdahiya]]

(In reply to Russ Nicoletti [:russn] from comment #8)
> Environment:
> 
> Gaia      3a06aa58245eaf848242d6d1497c1af536fffabd
> Gecko     https://hg.mozilla.org/mozilla-central/rev/6c0971104909
> BuildID   20140725040205
> Version   34.0a1
> Device    hamachi
> 
> To reproduce, I've found is that I need to do the following:
> 
> * When gallery app is coming up and thumbnails are being displayed, quickly
> tap on select button, then a photo, then delete.
> 
> For a brief moment, the small 'cancel' and 'delete' button appear at the top
> left of the screen. They appear for less time on hamachi than on flame. I've
> found the "small" buttons appear longer on flame.
> 
> I don't see any gallery info when I am running logcat.

Thanks Russ, I am able to replicate the delete and confirm button displaying without css for fraction of seconds before showing confirm dialog using above steps. This happens only on first delete click and all the subsequent click seems fine.

I doubt this is regression of bug 1011772 as we were lazy loading confirm.css in apps/gallery/js/dialogs.js. I tested this on Build Id 20140715160202 and able to replicate this issue. This issue should be existing since January when bug 949857 landed.

Comment 11

[Punam Dahiya [:pdahiya]]

(In reply to Russ Nicoletti [:russn] from comment #9)
> (In reply to Punam Dahiya from comment #6)
> > (In reply to Russ Nicoletti [:russn] from comment #5)
> > > I am able to reproduce with Sireesha's STR. Her solution of uncommenting
> > > confirm.css in gallery index.html seems fine as video index.html includes
> > > confirm.css as well.
> > 
> > Hi Russ
> > 
> > I tried to replicate again this time on Hamachi (buildid 20140724160202)
> > following below steps from #comment0 , but confirm delete dialog shows fine. 
> > 
> > open Gallery->Click on Camera->Take more than 50 pictures and videos->Click
> > on Home button and 
> > Open->Gallery->Click on Select button(dotted box)->Select 2 to 3
> > images/video->Click on Delete icon
> > 
> > IMO, We should load confirm.css only when needed,in this case inside
> > shared/js/dialogs.js.
> > 
> > In video app, we should comment confirm.css in index.html. However we are
> > using confirm.css in showOverlay inside video.js. To not break overlays
> > 
> > 1. One solution is to lazy load confirm.css in showOverlay inside video.js
> > 
> > 2. Another approach, is to remove showOverlay method from video.js and use
> > Dialogs.showOverlay(). IMO a better fix, wdyt?
> 
> I like the idea of using Dialogs.showOverlay because that would remove what
> is basically duplicate code from video app. I will make that change on my
> box and test it out. 
> 
> However, it seems the issue with confirm.css will still exist. See comment
> #8.

Agreed, may be we should do Dialogs.showOverlay refactoring in video app as a separate bug and keep this bug for original issue in comment #8 for triage and track the fix.

Comment 12

[Russ Nicoletti [:russn]]

(In reply to Punam Dahiya from comment #11)
> (In reply to Russ Nicoletti [:russn] from comment #9)
> > (In reply to Punam Dahiya from comment #6)
> > > (In reply to Russ Nicoletti [:russn] from comment #5)
> > > > I am able to reproduce with Sireesha's STR. Her solution of uncommenting
> > > > confirm.css in gallery index.html seems fine as video index.html includes
> > > > confirm.css as well.
> > > 
> > > Hi Russ
> > > 
> > > I tried to replicate again this time on Hamachi (buildid 20140724160202)
> > > following below steps from #comment0 , but confirm delete dialog shows fine. 
> > > 
> > > open Gallery->Click on Camera->Take more than 50 pictures and videos->Click
> > > on Home button and 
> > > Open->Gallery->Click on Select button(dotted box)->Select 2 to 3
> > > images/video->Click on Delete icon
> > > 
> > > IMO, We should load confirm.css only when needed,in this case inside
> > > shared/js/dialogs.js.
> > > 
> > > In video app, we should comment confirm.css in index.html. However we are
> > > using confirm.css in showOverlay inside video.js. To not break overlays
> > > 
> > > 1. One solution is to lazy load confirm.css in showOverlay inside video.js
> > > 
> > > 2. Another approach, is to remove showOverlay method from video.js and use
> > > Dialogs.showOverlay(). IMO a better fix, wdyt?
> > 
> > I like the idea of using Dialogs.showOverlay because that would remove what
> > is basically duplicate code from video app. I will make that change on my
> > box and test it out. 
> > 
> > However, it seems the issue with confirm.css will still exist. See comment
> > #8.
> 
> Agreed, may be we should do Dialogs.showOverlay refactoring in video app as
> a separate bug and keep this bug for original issue in comment #8 for triage
> and track the fix.

Bug 1045265 created to track video app using Dialogs.showOverlay

Comment 13

[ranjith253]

By adding a setTimeout for showing the dialog I was able to fix this issue.
I tested in all three apps Music , gallery & video not able to reproduce the issue.

 setTimeout(function() {
  // show the confirm dialog
  dialog.classList.remove('hidden');
});

Comment 14

[BMO Automation]

Firefox OS is not being worked on