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

RESOLVED WONTFIX

Status

RESOLVED WONTFIX
4 years ago
8 months ago

People

(Reporter: eva.chen.fx, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Flatfish][TCP])

Attachments

(5 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
Created attachment 8480534 [details]
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
(Reporter)

Updated

4 years ago
Blocks: 903304
Whiteboard: [Flatfish][TCP]
First broken gaia: aeba863d5cf8c9d170d46b20b77f99aae5f90c93
Caused by bug 1019841?
See Also: → bug 1019841
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?
Attachment #8482118 - Flags: feedback?(dmarcos)
Created attachment 8482464 [details] [review]
Pull request
Attachment #8482118 - Attachment is obsolete: true
Attachment #8482118 - Flags: feedback?(dmarcos)
Attachment #8482464 - Flags: review?(dflanagan)
Attachment #8482464 - Flags: feedback?(wilsonpage)
Attachment #8482464 - Flags: feedback?(dmarcos)
Created attachment 8482467 [details]
GalleryPreview-before.png
Created attachment 8482468 [details]
GalleryPreview-after.png
(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.
Flags: needinfo?(gduan)
Attachment #8482464 - Flags: feedback?(dmarcos)

Comment 7

4 years ago
(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?
Status: UNCONFIRMED → NEW
Ever confirmed: true
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
Attachment #8482464 - Flags: review?(dflanagan) → review+
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.)
Flags: needinfo?(wilsonpage)
Flags: needinfo?(gduan)
Flags: needinfo?(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.
Flags: needinfo?(dmarcos) → needinfo?(shchen)
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
Attachment #8482464 - Flags: feedback?(wilsonpage) → feedback-
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.
Flags: needinfo?(shchen)
Created attachment 8483087 [details] [review]
PR Adding comment to explain why !important declaration is necessary
Attachment #8483087 - Flags: review?(dflanagan)
Comment on attachment 8482464 [details] [review]
Pull request

PR updated and comment addressed, please take a look and give me some feedback, thanks.
Attachment #8482464 - Flags: feedback- → feedback?(dmarcos)
Duplicate of this bug: 1062030
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.
Attachment #8483087 - Flags: review?(dflanagan) → review+
I can repro this bug in today's build (2.2.0.0-prerelease,  Version : 35.0a1, BuildID : 20140905012648
When bug 992249 lands we won't need !important any longer.
Flags: needinfo?(wilsonpage)
See Also: → bug 992249
Attachment #8482464 - Flags: feedback?(dmarcos)

Comment 20

8 months ago
Firefox OS is not being worked on
Status: NEW → RESOLVED
Last Resolved: 8 months ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.