1059748 - [Flatfish][Gallery] There are two issues in the Gallery app.

Status: RESOLVED WONTFIX

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

Description

[eva.chen.fx]

Attachment: bug_[Gallery]wrong display.jpg

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/36.0.1985.143 Safari/537.36

Steps to reproduce:

Repro frequency: 100%
Prerequisite: There are some photos in the Gallery.
1. Open the Gallery app.
2. Choose a photo
<ISSUE>
First issue: When selecting the first thumbnail of the photo, there is second photo showing on the right side of the screen; when selecting second, there is third showing......and so on.
See attached: bug_[Gallery]wrong display.jpg
Second issue: The full screen function do not work.

------------------------------------------------------
* "Broken" Env. Info:
  Device  : Flatfish 2.1 Master
  B2G      : 2.1.0.0-prerelease
  Gaia     : b5aed82b1d8750a53848479b6400902d920d1475
               Merge: 553f796 7a06c70
               Date:   Thu Aug 28 08:21:36 2014 +0200
  Gecko   : 64c4bef1c1234d2fdd60974ed30d1034ec570c34
  BuildID : 20140821013807
  Version : 34.0a1
  
* "Working" Env. Info:
  Device  : Flatfish 2.1 Master
  B2G     : 2.1.0.0-prerelease
  Gaia    : 4ccb207518f62baa5160a735a6189d3ba9d34e1d
  Gecko   : 64c4bef1c1234d2fdd60974ed30d1034ec570c34
  BuildID : 20140821013807
  Version : 34.0a1

Comment 1

[Sherman Chen [:chens] (inactive)]

First broken gaia: aeba863d5cf8c9d170d46b20b77f99aae5f90c93
Caused by bug 1019841?

Comment 2

[Sherman Chen [:chens] (inactive)]

Attachment: WIP - part 1

Hi Diego, 
May I know the reason why we use !important for .large-only style? Trying to follow comments in bug 1019841 but didn't find the reason, is there any particular style it will fail?

Comment 3

[Sherman Chen [:chens] (inactive)]

Attachment: Pull request

Comment 4

[Sherman Chen [:chens] (inactive)]

Attachment: GalleryPreview-before.png

Comment 5

[Sherman Chen [:chens] (inactive)]

Attachment: GalleryPreview-after.png

Comment 6

[Diego Marcos [:dmarcos]]

(In reply to Sherman Chen [:chens] from comment #2)
> Created attachment 8482118 [details] [review]
> WIP - part 1
> 
> Hi Diego, 
> May I know the reason why we use !important for .large-only style? Trying to
> follow comments in bug 1019841 but didn't find the reason, is there any
> particular style it will fail?

I haven't been involved on any of the flatfish effort. I believe George has worked on it and might have an answer.

Comment 7

[YF (Yang)]

(In reply to eva.chen.fx from comment #0)
> <ISSUE>
> First issue: When selecting the first thumbnail of the photo, there is
> second photo showing on the right side of the screen; when selecting second,
> there is third showing......and so on.
> See attached: bug_[Gallery]wrong display.jpg

I have encountered the issue on my tablet.

> Second issue: The full screen function do not work.

How to reproduce?

Comment 8

[David Flanagan [:djf]]

Comment on attachment 8482464 [details] [review]
Pull request

I noted some nits on github, but r+ to land if you fix those.

I'd prefer a fix, however that did not use !important, so I think we should investigate further whether it is possible to remove the !important declaration that was added in bug 1019841

Comment 9

[David Flanagan [:djf]]

Diego,

In comment #2, Sherman is asking you why you added an !important declaration in gallery.css as part of bug 1019841. That was the cause of this regression and Sherman wants to know if he can remove it. Your response in comment #6 does not answer Sherman's question and it seems like you misunderstood.

Given the history of bug 1019841, that line was probably part of Wilson's original patch so he may have a better answer for Sherman.  So setting needinfo for both Diego and Wilson and clearing the needinfo on George.  (Note, however that Wilson is OOO this week.)

Comment 10

[Diego Marcos [:dmarcos]]

Sherman, the !important declaration is necessary to workaround the higher specificity of the styles applied within the gaia-header web component. If you remove !important we cannot style the component from the outside since internal rules have precedence over the external ones. This is one of the remaining rough edges around the web components implementation.

Comment 11

[Diego Marcos [:dmarcos]]

Comment on attachment 8482464 [details] [review]
Pull request

The !important declaration was introduce by bug 1019841. Explanation here:

https://bugzilla.mozilla.org/show_bug.cgi?id=1019841#c28

gallery.css is already excluded for the linter in:

https://github.com/mozilla-b2g/gaia/blob/master/build/csslint/xfail.list#L38

Comment 12

[Sherman Chen [:chens] (inactive)]

Diego, thanks for the explanation, and I've also notice that workaround is necessary. However if we use !important to hide header on phone view, we cannot avoid using !important to make it visible on tablet view. That's why I didn't remove it in gallery.css, but put another !important in gallery_table.css.

Comment 13

[Diego Marcos [:dmarcos]]

Attachment: PR Adding comment to explain why !important declaration is necessary

Comment 14

[Sherman Chen [:chens] (inactive)]

Comment on attachment 8482464 [details] [review]
Pull request

PR updated and comment addressed, please take a look and give me some feedback, thanks.

Comment 16

[David Flanagan [:djf]]

Comment on attachment 8483087 [details] [review]
PR Adding comment to explain why !important declaration is necessary

I'm always in favor of more comments! If there is a web components bug filed about the issue of needing the !important declaration, then you might reference that bug number in your comment so we'll know when we can remove !important.

Comment 17

[Gabriela [:gaby2300]]

I can repro this bug in today's build (2.2.0.0-prerelease,  Version : 35.0a1, BuildID : 20140905012648

Comment 18

[Wilson Page [:wilsonpage]]

When bug 992249 lands we won't need !important any longer.

Comment 19

[Sherman Chen [:chens] (inactive)]

Merged to master: https://github.com/mozilla-b2g/gaia/commit/b630b8bcaf9653885539d4449bc65c3b592bd752

Comment 20

[BMO Automation]

Firefox OS is not being worked on