Launchpad itself

Merge lp://staging/~huwshimi/launchpad/privacy-notification-firefox-753423 into lp://staging/launchpad

privacy-notification-firefox-753423
Merge into devel

Proposed by Huw Wilkins on 2011-04-08

Status:

Merged

Approved by:

Huw Wilkins on 2011-04-14

Approved revision:

no longer in the source branch.

Merged at revision:

12883

Proposed branch:

lp://staging/~huwshimi/launchpad/privacy-notification-firefox-753423

Merge into:

lp://staging/launchpad

Diff against target:

30 lines (+3/-3)

1 file modified

lib/lp/bugs/javascript/bugtask_index.js (+3/-3)

To merge this branch:

bzr merge lp://staging/~huwshimi/launchpad/privacy-notification-firefox-753423

High

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Henning Eggers (community)	code	2011-04-08	Approve on 2011-04-11
Review via email: mp+56872@code.staging.launchpad.net

Commit message

[r=henninge][bug=753423] Rewrote privacy notification spacing to fix YUI3 Firefox issues.

Description of the change

Privacy notifications were broken in Firefox due to YUI crashing when trying to animate certain CSS properties in Firefox. I have rewritten how it goes about creating a space at the top of the page to use working parts of YUI.

Revision history for this message

Henning Eggers (henninge) wrote on 2011-04-11:

Thanks for fixing this. It's a pity though that this could only be fixed by using setStyle. The class approach is so much cleaner.

There is still a little glitch in Firefox. Set a bug to "private", dismiss the notification at the top and the background color of "This bug report is private" box changes. Now change it back to public and the box will briefly turn white again, only to return to dark red again (but the text remains black).

Finally, a note about the cover letter. I would have appreciated more context information and most importantly a hint about the necessary feature flag. Also a demo URL would have been nice. I could only gather that this is on a bugs page by reading the diff. Do you know the cover letter template like it is produced by "bzr lp-propose"?

Still, the code looks good. Although the functionality still needs fixing before the feature can go public.

review: Approve (code)

Revision history for this message

Huw Wilkins (huwshimi) wrote on 2011-04-13:

I figured out the issue was actually that Firefox only understands CSS properties that have been converted to camel case. I've reverted all my changes and have fixed it with this simple change.

> Thanks for fixing this. It's a pity though that this could only be fixed by
> using setStyle. The class approach is so much cleaner.

It's back to using classes.

> There is still a little glitch in Firefox. Set a bug to "private", dismiss the
> notification at the top and the background color of "This bug report is
> private" box changes. Now change it back to public and the box will briefly
> turn white again, only to return to dark red again (but the text remains
> black).

Fixed.

> Finally, a note about the cover letter. I would have appreciated more context
> information and most importantly a hint about the necessary feature flag. Also
> a demo URL would have been nice. I could only gather that this is on a bugs
> page by reading the diff. Do you know the cover letter template like it is
> produced by "bzr lp-propose"?

Sorry about that I had been talking to someone about the issue and was hoping they would review it. I guess I was being lazy.

I did not know about lp-propse. I'll take a look.