631257 - Only default snippets are styled in Firefox Start Page design

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[Les Orchard [:lorchard]]

The new Firefox Start Page accommodates snippet content pulled from a web service (bug 592431). When the service is available, the children of div#defaultSnippets are hidden and service content is injected into div#snippetContainer

So, updates to the Start Page design (bug 627301) have two problems in this scenario:

1) Only the #defaultSnippets div has the new styling for the snippets box, so injected snippets are left unstyled.

2) The child elements of #defaultSnippets are hidden, but the empty parent div stays visible.

Comment 1

[Marco Bonardo [:mak]]

(In reply to comment #0)
> 1) Only the #defaultSnippets div has the new styling for the snippets box, so
> injected snippets are left unstyled.

right, the styling should be applied to both or to the container.

> 2) The child elements of #defaultSnippets are hidden, but the empty parent div
> stays visible.

right, should be collapsed.

Comment 2

[Les Orchard [:lorchard]]

Attachment: screenshot of snippet from service

Comment 3

[Les Orchard [:lorchard]]

Attachment: snippet HTML from service

Comment 4

[Les Orchard [:lorchard]]

Attachment: snippet HTML from service

Here's a sample of the HTML from the snippet service. It has 3 snippets, with only one revealed at random by javascript.

Haven't really styled the snippets, since I'm not sure if that styling should be a part of aboutHome.css or part of the snippet payload. So, the icons are just kind of out there

Comment 5

[Les Orchard [:lorchard]]

FWIW: I can hack around this in JS by moving the DOM node of a visible injected snippet into #defaultSnippets, but I'd like to not have to do that.

Comment 6

[Marco Bonardo [:mak]]

should not be hard to fix

Comment 7

[Laura Forrest]

Marco - how's this fix going? We're aiming to launch this service with Beta 11.

Comment 8

[Marco Bonardo [:mak]]

(In reply to comment #7)
> Marco - how's this fix going? We're aiming to launch this service with Beta 11.

beta11 has already been signed off, this fix cannot make it...

Comment 9

[Marco Bonardo [:mak]]

Asking softblocking in the meanwhile, if we want snippets this is most likely a must-have fix.

Comment 10

[Les Orchard [:lorchard]]

(In reply to comment #9)
> Asking softblocking in the meanwhile, if we want snippets this is most likely a
> must-have fix.

We do want snippets, and we're aiming to push some for beta 11. 

But, I think we can have snippets in beta 11, just with a hack-around for this design bug. The snippet code is still there in aboutHome.js, right? So, if snippets.mozilla.com goes live, beta 11 will request content?

Comment 11

[Marco Bonardo [:mak]]

yep, the difference is b11 is listening to http, while b12 will be on https (from tomorrow's nightly)

Comment 12

[Marco Bonardo [:mak]]

Attachment: patch v1.0

I've simplified a bit the thing, to avoid having to manage separate styles.
"defaultSnippets" is always a hidden div with <span> default snippets in it.
"snippets" is the only styled and shown container, for both remote or local snippets.
When there are no valid remote snippets, the code takes a random <span> from "defaultSnippets" and injects it into "snippets".

Regarding the big button click, in the defaults snippets case it's easy to hook, since there is just one span with a link. In the remote snippets case, I cannot know where the link is (there is also a <script> involved), so I give you a activateSnippetsButtonClick(aElt) helper. The remote <script> will have to call it on the choosen snippet element, it doesn't matter that it's exactly inside that element (the search is recursive) but the passed-in element should have only 1 link into it.
In the example you posted above, you could just call activateSnippetsButtonClick(show_snippet);

Clearly this has still to pass the review process and names could change, but that's my basic idea, does this work for you?

Comment 13

[Les Orchard [:lorchard]]

(In reply to comment #12)

> Regarding the big button click, in the defaults snippets case it's easy to
> hook, since there is just one span with a link. In the remote snippets case, I
> cannot know where the link is (there is also a <script> involved), so I give
> you a activateSnippetsButtonClick(aElt) helper. The remote <script> will have
> to call it on the choosen snippet element, it doesn't matter that it's exactly
> inside that element (the search is recursive) but the passed-in element should
> have only 1 link into it.
> In the example you posted above, you could just call
> activateSnippetsButtonClick(show_snippet);
> 
> Clearly this has still to pass the review process and names could change, but
> that's my basic idea, does this work for you?

I think this is more a question for Engagement than me - this space is ultimately for the content they have planned.

Will all snippets just be one big link? If so, this might work.

But, if snippets are intended to have multiple links or even other interactive elements, then we might run into problems.

Comment 14

[Marco Bonardo [:mak]]

(In reply to comment #13)
> But, if snippets are intended to have multiple links or even other interactive
> elements, then we might run into problems.

It's up to your script to decide to call activateSnippetsButtonClick or not, you can avoid calling it and provide your own replacement. I don't linkify automatically the button exactly because I don't know what it contains.

Comment 15

[Marco Bonardo [:mak]]

and actually calling the helper on something that has more or less than 1 link is a no-op.

Comment 16

[Laura Forrest]

(In reply to comment #13)
> (In reply to comment #12)
> I think this is more a question for Engagement than me - this space is
> ultimately for the content they have planned.
> 
> Will all snippets just be one big link? If so, this might work.
> 
> But, if snippets are intended to have multiple links or even other interactive
> elements, then we might run into problems.

From the engagement perspective our first priority is to establish feature parity to the current snippets. Those contain copy, a graphic, and one link. 

However, we would like to incorporate other enhancements like multiple links, interactive elements like a sign-up form field, and more in the future so it's important to make changes with that in mind.

Comment 17

[Marco Bonardo [:mak]]

Mike the whiteboard [softblock] value seems bogus, platform meeting queries use the full [softblocker] string...

Btw, I think this is not that much soft, I'm not comfortable at all having the final product homepage rely on a remote hack to show snippets.

Comment 18

[Alex Faaborg [:faaborg] (Firefox UX)]

Nominating this to move from softblocker to hardblocker, since we can't really ship a home page that looks like this: 

https://bugzilla.mozilla.org/attachment.cgi?id=509469

Comment 19

[Marco Bonardo [:mak]]

Thanks Alex, I was going to do the same.
From an engineering point of view I'm scared that remote snippets have to add some complicated code to workaround this bug.  It will be one of the most accessed pages.

Comment 20

[Laura Forrest]

(In reply to comment #19)
> From an engineering point of view I'm scared that remote snippets have to add
> some complicated code to workaround this bug.  It will be one of the most
> accessed pages.

Agree that a workaround isn't desirable. As we're aiming for FxB 12 to launch this service can you give us an idea of what needs to be done so that a workaround won't be needed?

Comment 21

[Marco Bonardo [:mak]]

(In reply to comment #20)
> can you give us an idea of what needs to be done so that a
> workaround won't be needed?

Review and land the patch in this bug.

Comment 22

[Mike Beltzner [:beltzner, not reading bugmail]]

This remains a soft blocker for final; if we don't get it for Firefox 4, then we can do a workaround on the server side or get the right fix out there in the first maintenance release.

I would not hold the product on this bug. That said, I also believe that it should be a high priority as a soft blocker. Just waiting on Gavin's review.

Comment 23

[Mehdi Mulani [:mmm] (I don't check this)]

http://hg.mozilla.org/mozilla-central/rev/346e58464d3d

Comment 24

[Mike Beltzner [:beltzner, not reading bugmail]]

msucan reported and I can confirm that the button-link-logic here seems to be going awry; can't tell if that's because of the way the snippet is being served up, or the way we're trying to detect the links, but the buttons generated by the  snippets that we're getting from the server look like you can click the entire button, but only the link goes somewhere.

Followup?

Comment 25

[Marco Bonardo [:mak]]

that's bug 634504, should start working from today's update.

Comment 26

[Teodosia Pop]

Verified using Mozilla/5.0 (Windows NT 6.1; rv:2.0b12) Gecko/20100101 Firefox/4.0b12.