Closed Bug 1059748 Opened 10 years ago Closed 7 years ago

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

Categories

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

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: eva.chen.fx, Unassigned)

References

Details

(Whiteboard: [Flatfish][TCP])

Attachments

(5 files, 1 obsolete file)

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
Blocks: flatfish
Whiteboard: [Flatfish][TCP]
First broken gaia: aeba863d5cf8c9d170d46b20b77f99aae5f90c93 Caused by bug 1019841?
See Also: → 1019841
Attached file WIP - part 1 (obsolete) —
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)
Attached file 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)
(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)
(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)
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)
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: → 992249
Attachment #8482464 - Flags: feedback?(dmarcos)
Firefox OS is not being worked on
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: