use collapsed instead of hidden for status panels

RESOLVED FIXED in Firefox 17

Status

()

Firefox
SocialAPI
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mixedpuppy, Assigned: mixedpuppy)

Tracking

unspecified
Firefox 17
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Fx17])

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

5 years ago
using hidden is changing the content so it is not showing up correctly.  using collapsed allows the content to retain its correct appearance.
(Assignee)

Comment 1

5 years ago
Created attachment 654446 [details] [diff] [review]
use collapsed
Attachment #654446 - Flags: review?(jaws)
The panel needs to stay initially hidden so as to not impact browser startup time.

Using collapsed for the iframes inside the panel makes sense, though. Is that alone sufficient to address the problems you're seeing?
(Assignee)

Comment 3

5 years ago
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #2)
> The panel needs to stay initially hidden so as to not impact browser startup
> time.
> 
> Using collapsed for the iframes inside the panel makes sense, though. Is
> that alone sufficient to address the problems you're seeing?

no, if the panel is hidden when we preload the the status panels the content doesn't render correctly.  but we can require providers to reset their sizes when they get the socialFrameShow event, then we can have panel hidden on startup.
(Assignee)

Updated

5 years ago
Duplicate of this bug: 785085
(Assignee)

Comment 5

5 years ago
Created attachment 654688 [details] [diff] [review]
use collapsed

keeps panel hidden for startup, content will need to draw on the socialFrameShow event.
Attachment #654446 - Attachment is obsolete: true
Attachment #654446 - Flags: review?(jaws)
Attachment #654688 - Flags: review?(jaws)
Attachment #654688 - Flags: review?(gavin.sharp)
We can unhide the panel before we preload the panels. We just can't have it unhidden by default (e.g. when social is entirely disabled).
(Assignee)

Comment 7

5 years ago
Created attachment 654702 [details] [diff] [review]
use collapsed

ok, this unhides the panel when updateButton is called, prior to setting the status iframes.  This works well, avoids the need for content to redraw the first time.
Attachment #654688 - Attachment is obsolete: true
Attachment #654688 - Flags: review?(jaws)
Attachment #654688 - Flags: review?(gavin.sharp)
Attachment #654702 - Flags: review?(gavin.sharp)
Comment on attachment 654702 [details] [diff] [review]
use collapsed

Please define |let panel = document.getElementById("social-notification-panel");| rather than using notifBox.parentNode - make it much clearer what element you're referring to (and makes it easy to grep for references to social-notification-panel if we're refactoring things later).

r=me with that.
Attachment #654702 - Flags: review?(gavin.sharp) → review+
(Assignee)

Comment 9

5 years ago
Created attachment 654710 [details] [diff] [review]
use collapsed

updated from comments
Attachment #654702 - Attachment is obsolete: true
Attachment #654710 - Flags: review?(gavin.sharp)
Comment on attachment 654710 [details] [diff] [review]
use collapsed

Hmm, actually I wonder whether we should continue to use "hidden" for the iframes. It reduces the memory footprint for unused documents, and it shouldn't have any impact on behavior?
(Assignee)

Comment 11

5 years ago
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #10)
> Comment on attachment 654710 [details] [diff] [review]
> use collapsed
> 
> Hmm, actually I wonder whether we should continue to use "hidden" for the
> iframes. It reduces the memory footprint for unused documents, and it
> shouldn't have any impact on behavior?

Using hidden reflows some of the content ui to zero height/width and requires content to deal with that when visible.  I think this will be a non-obvious issue for providers in general.  Let me know what you want to do.
I don't think hiding and then unhiding an iframe should have any side effects. Can you describe the problem you were seeing in more detail?
Comment on attachment 654710 [details] [diff] [review]
use collapsed

This is fine if it fixes the immediate problem. I want to investigate more later to see if we can somehow get away with hiding entirely rather than just collapsing, but that might be difficult given how we keep the content preloaded.
Attachment #654710 - Flags: review?(gavin.sharp) → review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
(Assignee)

Comment 14

5 years ago
https://tbpl.mozilla.org/?tree=Try&rev=77badc2b71dc
https://hg.mozilla.org/integration/mozilla-inbound/rev/aa807b020b71
Status: NEW → ASSIGNED
Flags: in-testsuite+
OS: Mac OS X → All
Hardware: x86 → All
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/aa807b020b71
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 17
You need to log in before you can comment on or make changes to this bug.