Merge lp:~mbp/launchpad/798412-plusone into lp:launchpad
- 798412-plusone
- Merge into devel
Status: | Rejected |
---|---|
Rejected by: | Robert Collins |
Proposed branch: | lp:~mbp/launchpad/798412-plusone |
Merge into: | lp:launchpad |
Diff against target: |
584 lines (+203/-35) 22 files modified
lib/canonical/launchpad/webapp/publisher.py (+11/-0) lib/lp/answers/stories/question-workflow.txt (+8/-3) lib/lp/answers/templates/question-index.pt (+7/-1) lib/lp/answers/templates/questionmessage-display.pt (+22/-4) lib/lp/app/templates/base-layout-macros.pt (+26/-0) lib/lp/app/templates/base-layout.pt (+15/-0) lib/lp/blueprints/browser/specification.py (+4/-0) lib/lp/blueprints/templates/specification-index.pt (+2/-0) lib/lp/bugs/browser/bugcomment.py (+4/-0) lib/lp/bugs/templates/bugcomment-box.pt (+18/-7) lib/lp/bugs/templates/bugcomment-index.pt (+1/-1) lib/lp/bugs/templates/bugtask-index.pt (+7/-2) lib/lp/code/browser/branchmergeproposal.py (+6/-1) lib/lp/code/stories/branches/xx-code-review-comments.txt (+3/-3) lib/lp/code/templates/branchmergeproposal-index.pt (+6/-0) lib/lp/code/templates/codereviewcomment-body.pt (+6/-4) lib/lp/code/templates/codereviewcomment-header.pt (+25/-2) lib/lp/registry/templates/distribution-index.pt (+6/-2) lib/lp/registry/templates/product-index.pt (+4/-2) lib/lp/services/comments/templates/comment.pt (+5/-1) lib/lp/services/features/flags.py (+6/-0) lib/lp/soyuz/templates/archive-index.pt (+11/-2) |
To merge this branch: | bzr merge lp:~mbp/launchpad/798412-plusone |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Robert Collins (community) | Disapprove | ||
Review via email: mp+83449@code.launchpad.net |
Commit message
Description of the change
This adds some Google Plus +1 buttons on various interesting objects in Launchpad: archive, product, distro, mp, bug, bug comment, answer, specs.
This could be very usefully extended to comments on bmps and answers, but the object needs to have a URL of its own and those don't yet.
We shouldn't add them to every single page, but mostly to things people are likely to express approval of or interest in. I added them on things I already see people sharing, and on things I feel like sharing myself.
There is a feature flag to turn this off. It defaults to on. We might eventually want a per-user opt-out, but people who feel strongly about not having this probably have a browser configuration to get that already.
There are no specific tests for it (yet). I think the most interesting test is really whether it works with the external systems.
I would like to also add this on bmp comments and answer comments, but the object has to have its own url and I think those don't yet.
I don't show the button on private objects, to prevent people accidentally sharing them, and (questionable) to prevent Google seeing the URL exists. This would be cleaner if there was a consistent .private on eg LaunchpadView. I might add one.
Possibly the button should be moved to a TAL macro.
Aaron Bentley (abentley) wrote : | # |
Curtis Hovey (sinzui) wrote : | # |
Questions comments do have a URL, but we have not registered a view to format them for the browser.
The URL is /<project>
Martin Pool (mbp) wrote : | # |
thanks guys, I will see about adding that.
Martin Pool (mbp) wrote : | # |
https:/
I can see zcml that tries to set it up...
Martin Pool (mbp) wrote : | # |
> Questions comments do have a URL, but we have not registered a view to format
> them for the browser.
> The URL is /<project>
> as /launchpad/
I had a go at trying to enable them, but I can't work it out :-(
Martin Pool (mbp) wrote : | # |
Ok, the updates here add buttons on merge proposals, though not yet on answers, and factor some of it out into macros used across various pages.
This doesn't yet add or update tests. It probably needs some, probably written looking at the resulting html.
The biggest bug I know of is that it does not stay up to date well when the page is updated by ajax, either for privacy or for adding new comments.
Robert Collins (lifeless) wrote : | # |
So, this has rather significant private-object implications.
See for instance the discussions we had on-list about trust of third
party js API's that get access to our objects, and also consider that
publicy stating a private object exists is an issue itself.
I think that we can probably tolerate the external js without a
contract / DPA analysis IF and only IF it is not loaded where
personally identifying information and / or private data is accessible
on the page.
I'm not sure how deep that rabbit hole will go; I suggest consulting
with curtis or deryck about this.
Robert Collins (lifeless) wrote : | # |
(I realise my reply may look like I hadn't read the MP. I had :)).
You cover directly some private objects but not e.g. branch merge
proposals or their comments as being private (or even hidden). So
there is a risk that folk adding this won't cater for privacy in some
contexts, in the current implementation. I'd like to see *something*
done to reduce or mitigate that.
And there is no (apparent) consideration for personally identifying
data (which is a vague concept at best, but consider for instance that
a script running in our context can access an email address that a
script running from a different site cannot.
We also have the trust issue, which really isn't about the code, but
about whether we can trust the google plus API code with private data
(and this applies to non-private pages because a script can make API
calls and access anything you can access).
Martin Pool (mbp) wrote : | # |
Hi,
I agree we need to get privacy (both data and people) right.
The benefit from all of this, hopefully, is
- more positive feedback for good work people do on Launchpad-hosted projects
- more visibility for Launchpad and projects in Launchpad on the web and on g+
- better search results about Launchpad
The biggest concern I have is not that Google will deploy malicious Javascript, but rather that people will accidentally click to share something that is meant to be private. So I want to hide the buttons on private pages -- and in fact I do, but it is not kept up to date after an ajax privacy change, but that can be done. We actually have belt-and-braces protection against that in that Google pings the page after you +1 it and it will refuse the plusone if the page is not accessible, which our private objects will be.
Showing the buttons in the page where we have control over them arguably makes it less likely people will accidentally click an external share button for a private object. It is more under our control.
My intention here is to provide, through the view.is_private check, a one-stop protection to make sure that these buttons are not rendered and the script is not loaded on views of private objects, without counting on people getting it right on each individual page. I think that means the current code will work ok even on bmps ... and I just tested, and in fact it does.
I ought to add tests that this is and stays correctly hooked up.
Personal data is hairy, arguably even including people's names, in which case every page of Launchpad is affected. What I'm trying to do here is to make it no worse than the current case combination of robots walking Launchpad public/anonymous pages, plus people sharing links through other means. The most relevant thing here is probably non-public email addresses. In a separate prior landing I add a meta description with the email addresses stripped out, so people shouldn't be accidentally sharing this. I'm also not putting this on any pages that are primarily about people, so the biggest risk is when personal information occurs within eg a public bug or mp description or comment.
To sum up the privacy requirements I am aiming for are:
* do not share any private objects
* do not encourage people to accidentally share things they shouldn't
* the framework should be safe by default for new development
* don't put email addresses into the shared content
* don't run 3rd party javascript on pages containing private content (any more than we currently do)
Martin Pool (mbp) wrote : | # |
There's some more discussion of this versus a builtin 'thanks' button in bug 798412.
I'm not sure about it.
It is smaller than a built in thanks mechanism would probably be.
On the other hand there is at least the perception of a privacy problem, perhaps an actual privacy problem, and perhaps ideally a builtin one would be better (though, we could always add this and switch later.)
Robert Collins (lifeless) wrote : | # |
Hi, sorry to need to reject this, but I've clarified some policy constraints IS have around externally hosted JS. I'll be working up proper policy docs etc soon, but the tl;dr is - we cannot run js that is hosted on a third party site in the LP webapp browser context.
Robert Collins (lifeless) wrote : | # |
One way to move this forward would be to investigate self hosted js that talks to the g+ servers - so a wire protocol rather than JS APIs.
Martin Pool (mbp) wrote : | # |
> Hi, sorry to need to reject this, but I've clarified some policy constraints IS have around externally hosted JS. I'll be working up proper policy docs etc soon, but the tl;dr is - we cannot run js that is hosted on a third party site in the LP webapp browser context.
That's fine. It was interesting and educational to do this, but there
are enough tradeoffs here that I'm not sure I even want to land it. I
can see now how we could fairly easily hook a similar design in to the
notification system, once that exists. The metadata/
changes make it easier for people to share pages using their own tools
if they want to.
There are some incidental cleanups in this branch that I will pick out
from the plusone bits.
I'm not being snarky by asking: does that policy mean that the
existing externally hosted scripts constitute bugs? Do we perhaps
want to at least squash them on private pages, using a similar check
to in this branch?
--
Martin
Robert Collins (lifeless) wrote : | # |
On Tue, Nov 29, 2011 at 11:39 AM, Martin Pool <email address hidden> wrote:
> I'm not being snarky by asking: does that policy mean that the
> existing externally hosted scripts constitute bugs? Do we perhaps
> want to at least squash them on private pages, using a similar check
> to in this branch?
They will have to go from all pages, because of the access they get.
Yes, they will constitute bugs.
-Rob
Martin Pool (mbp) wrote : | # |
On 29 November 2011 09:24, Robert Collins <email address hidden> wrote:
> One way to move this forward would be to investigate self hosted js that talks to the g+ servers - so a wire protocol rather than JS APIs.
I suspect that would be breaching their TOS, eg "You agree not to
access (or attempt to access) any of the Services by any means other
than through the interface that is provided by Google", and also that
it may be technically fragile.
It wouldn't deal with user concerns about tracking.
Perhaps we can just implement our own "thanks" concept more directly.
--
Martin
Martin Pool (mbp) wrote : | # |
everything but the plusone bits is now in https:/
Preview Diff
1 | === modified file 'lib/canonical/launchpad/webapp/publisher.py' |
2 | --- lib/canonical/launchpad/webapp/publisher.py 2011-11-25 02:19:28 +0000 |
3 | +++ lib/canonical/launchpad/webapp/publisher.py 2011-11-27 22:43:42 +0000 |
4 | @@ -75,6 +75,9 @@ |
5 | IStructuredString, |
6 | NoCanonicalUrl, |
7 | ) |
8 | +from canonical.launchpad.interfaces.launchpad import ( |
9 | + IPrivacy, |
10 | + ) |
11 | from canonical.launchpad.webapp.url import urlappend |
12 | from canonical.launchpad.webapp.vhosts import allvhosts |
13 | from lp.app.errors import NotFoundError |
14 | @@ -83,6 +86,7 @@ |
15 | defaultFlagValue, |
16 | getFeatureFlag, |
17 | ) |
18 | +from lp.services.propertycache import cachedproperty |
19 | from lp.services.utils import obfuscate_structure |
20 | |
21 | # Monkeypatch NotFound to always avoid generating OOPS |
22 | @@ -397,6 +401,13 @@ |
23 | cache, cls=ResourceJSONEncoder, |
24 | media_type=EntryResource.JSON_TYPE) |
25 | |
26 | + @cachedproperty |
27 | + def is_private(self): |
28 | + """True if the object is private. |
29 | + """ |
30 | + privacy = IPrivacy(self.context, None) |
31 | + return privacy is not None and privacy.private |
32 | + |
33 | def publishTraverse(self, request, name): |
34 | """See IBrowserPublisher.""" |
35 | # By default, a LaunchpadView cannot be traversed through. |
36 | |
37 | === modified file 'lib/lp/answers/stories/question-workflow.txt' |
38 | --- lib/lp/answers/stories/question-workflow.txt 2011-10-20 00:53:01 +0000 |
39 | +++ lib/lp/answers/stories/question-workflow.txt 2011-11-27 22:43:42 +0000 |
40 | @@ -291,7 +291,8 @@ |
41 | True |
42 | |
43 | >>> bestAnswer.first('div', 'boardCommentBody') |
44 | - <div class="boardCommentBody"><p>New version of the firefox package |
45 | + <div class="boardCommentBody" itemprop="commentText"><p>New version |
46 | + of the firefox package |
47 | are available with SVG support enabled. You can use apt-get or adept to |
48 | upgrade.</p></div> |
49 | |
50 | @@ -355,13 +356,17 @@ |
51 | >>> print extract_text(answerer) |
52 | No Privileges Person (no-priv) |
53 | |
54 | - >>> message = bestAnswer.parent.parent.find( |
55 | + >>> message = soup.find( |
56 | ... 'div', 'boardCommentBody highlighted') |
57 | + >>> print message |
58 | + <div class="boardCommentBody highlighted" |
59 | + itemprop="commentText"><p>New version of the firefox package are |
60 | + available with SVG support enabled. You can use apt-get or adept to |
61 | + upgrade.</p></div> |
62 | >>> print extract_text(message) |
63 | New version of the firefox package are available with SVG support |
64 | enabled. You can use apt-get or adept to upgrade. |
65 | |
66 | - |
67 | History |
68 | ======= |
69 | |
70 | |
71 | === modified file 'lib/lp/answers/templates/question-index.pt' |
72 | --- lib/lp/answers/templates/question-index.pt 2011-08-20 14:14:26 +0000 |
73 | +++ lib/lp/answers/templates/question-index.pt 2011-11-27 22:43:42 +0000 |
74 | @@ -31,6 +31,13 @@ |
75 | </metal:block> |
76 | </head> |
77 | <body> |
78 | + <metal:block |
79 | + metal:fill-slot="heading"> |
80 | + <tal:plusone |
81 | + metal:use-macro="context/@@+base-layout-macros/header-plusone-button" /> |
82 | + <h1 itemprop="name" tal:content="view/label"></h1> |
83 | + </metal:block> |
84 | + |
85 | <metal:registering fill-slot="registering"> |
86 | Asked by |
87 | <a tal:replace="structure context/owner/fmt:link" /> |
88 | @@ -59,7 +66,6 @@ |
89 | <div metal:fill-slot="main"> |
90 | <tal:description |
91 | define="global description context/description/fmt:obfuscate-email/fmt:text-to-html" /> |
92 | - |
93 | <div class="report" |
94 | tal:content="structure description"/> |
95 | |
96 | |
97 | === modified file 'lib/lp/answers/templates/questionmessage-display.pt' |
98 | --- lib/lp/answers/templates/questionmessage-display.pt 2011-05-31 14:58:26 +0000 |
99 | +++ lib/lp/answers/templates/questionmessage-display.pt 2011-11-27 22:43:42 +0000 |
100 | @@ -3,10 +3,16 @@ |
101 | xmlns:metal="http://xml.zope.org/namespaces/metal" |
102 | omit-tag=""> |
103 | <div |
104 | + itemscope="" |
105 | + itemtype="http://schema.org/UserComments" |
106 | tal:define="css_classes view/getBoardCommentCSSClass" |
107 | tal:attributes="class string:${css_classes}; |
108 | id string:comment-${context/index}"> |
109 | <div class="boardCommentDetails"> |
110 | + <table> |
111 | + <tbody> |
112 | + <tr> |
113 | + <td> |
114 | <tal:bestanswer condition="view/isBestAnswer"> |
115 | <img src="/@@/favourite-yes" style="float:right;" alt="Best" |
116 | title="Marked as best answer" /> |
117 | @@ -15,16 +21,28 @@ |
118 | <tal:comment_owner replace="structure context/owner/fmt:link-display-name-id" /> |
119 | </tal:comment_has_owner> |
120 | said |
121 | - <span |
122 | - tal:attributes="title context/datecreated/fmt:datetime" |
123 | + <time |
124 | + itemprop="commentTime" |
125 | + tal:attributes="title context/datecreated/fmt:datetime; |
126 | + datetime context/datecreated/fmt:isodate" |
127 | tal:content="context/datecreated/fmt:displaydate">Thursday |
128 | - 13:21</span>: |
129 | - <span style="float:right;" |
130 | + 13:21</time>: |
131 | + </td> |
132 | + <td tal:condition="nothing"> |
133 | + <!-- disabled until question messages have a working individual URL --> |
134 | + <tal:plusone |
135 | + metal:use-macro="context/@@+base-layout-macros/comment-plusone-button" /> |
136 | + </td> |
137 | + <td class="bug-comment-index"> |
138 | + <a |
139 | tal:content="string: #${context/display_index}" /> |
140 | + </td> |
141 | + </tr></tbody></table> |
142 | </div> |
143 | |
144 | <div class="boardCommentBody" |
145 | tal:attributes="class view/getBodyCSSClass" |
146 | + itemprop="commentText" |
147 | tal:content="structure |
148 | context/text_contents/fmt:obfuscate-email/fmt:email-to-html"> |
149 | Message text. |
150 | |
151 | === modified file 'lib/lp/app/templates/base-layout-macros.pt' |
152 | --- lib/lp/app/templates/base-layout-macros.pt 2011-11-15 21:04:08 +0000 |
153 | +++ lib/lp/app/templates/base-layout-macros.pt 2011-11-27 22:43:42 +0000 |
154 | @@ -314,4 +314,30 @@ |
155 | l_plural plural | string:$singular$l_default;" |
156 | condition="python: count != 1" |
157 | replace="l_plural" /></metal:plural-msg> |
158 | + |
159 | + |
160 | +<metal:comment-plusone-button |
161 | + define-macro="comment-plusone-button"> |
162 | + <div |
163 | + class="g-plusone" |
164 | + data-size="small" |
165 | + tal:attributes="href context/fmt:url" |
166 | + tal:condition="not:view/is_private" |
167 | + align="right" |
168 | + data-align="right"> |
169 | + </div> |
170 | +</metal:comment-plusone-button> |
171 | + |
172 | +<metal:header-plusone-button |
173 | + define-macro="header-plusone-button"> |
174 | + <div |
175 | + style="float:right" |
176 | + tal:condition="not:view/is_private"> |
177 | + <div |
178 | + class="g-plusone" |
179 | + tal:attributes="href context/fmt:url" |
180 | + align="right"> |
181 | + </div> |
182 | + </div> |
183 | +</metal:header-plusone-button> |
184 | </macros> |
185 | |
186 | === modified file 'lib/lp/app/templates/base-layout.pt' |
187 | --- lib/lp/app/templates/base-layout.pt 2011-11-25 02:19:28 +0000 |
188 | +++ lib/lp/app/templates/base-layout.pt 2011-11-27 22:43:42 +0000 |
189 | @@ -172,6 +172,21 @@ |
190 | |
191 | <metal:lp-client-cache |
192 | use-macro="context/@@+base-layout-macros/lp-client-cache" /> |
193 | + |
194 | + <tal:comment tal:replace="nothing"> |
195 | + When plusone is disabled, the div tags can still be emitted in to the page by |
196 | + other templates, but they will have no effect because the script is inactive. |
197 | + </tal:comment> |
198 | + <script |
199 | + tal:condition="not:request/features/plusone.buttons.disabled" |
200 | + type="text/javascript"> |
201 | + (function() { |
202 | + var po = document.createElement('script'); po.type = 'text/javascript'; po.async = true; |
203 | + po.src = 'https://apis.google.com/js/plusone.js'; |
204 | + var s = document.getElementsByTagName('script')[0]; s.parentNode.insertBefore(po, s); |
205 | + })(); |
206 | + </script> |
207 | + |
208 | </body> |
209 | |
210 | <tal:template> |
211 | |
212 | === modified file 'lib/lp/blueprints/browser/specification.py' |
213 | --- lib/lp/blueprints/browser/specification.py 2011-07-14 12:44:48 +0000 |
214 | +++ lib/lp/blueprints/browser/specification.py 2011-11-27 22:43:42 +0000 |
215 | @@ -562,6 +562,10 @@ |
216 | def page_title(self): |
217 | return self.label |
218 | |
219 | + @property |
220 | + def page_description(self): |
221 | + return self.context.summary |
222 | + |
223 | def initialize(self): |
224 | # The review that the user requested on this spec, if any. |
225 | self.notices = [] |
226 | |
227 | === modified file 'lib/lp/blueprints/templates/specification-index.pt' |
228 | --- lib/lp/blueprints/templates/specification-index.pt 2011-08-25 06:32:24 +0000 |
229 | +++ lib/lp/blueprints/templates/specification-index.pt 2011-11-27 22:43:42 +0000 |
230 | @@ -24,6 +24,8 @@ |
231 | </tal:registering> |
232 | |
233 | <metal:heading fill-slot="heading"> |
234 | + <tal:plusone |
235 | + metal:use-macro="context/@@+base-layout-macros/header-plusone-button" /> |
236 | <h1 tal:replace="structure view/title_widget"> |
237 | blueprint title |
238 | </h1> |
239 | |
240 | === modified file 'lib/lp/bugs/browser/bugcomment.py' |
241 | --- lib/lp/bugs/browser/bugcomment.py 2011-11-16 23:46:01 +0000 |
242 | +++ lib/lp/bugs/browser/bugcomment.py 2011-11-27 22:43:42 +0000 |
243 | @@ -341,6 +341,10 @@ |
244 | self.comment.index, self.context.bug.id) |
245 | |
246 | @property |
247 | + def page_description(self): |
248 | + return self.comment.text_contents |
249 | + |
250 | + @property |
251 | def privacy_notice_classes(self): |
252 | if not self.context.bug.private: |
253 | return 'hidden' |
254 | |
255 | === modified file 'lib/lp/bugs/templates/bugcomment-box.pt' |
256 | --- lib/lp/bugs/templates/bugcomment-box.pt 2011-11-24 00:37:40 +0000 |
257 | +++ lib/lp/bugs/templates/bugcomment-box.pt 2011-11-27 22:43:42 +0000 |
258 | @@ -36,23 +36,33 @@ |
259 | <td tal:condition="comment/bugwatch"> |
260 | <img width="14" height="14" src="/@@/bug-remote" alt=""/> |
261 | In |
262 | - <a tal:attributes="href comment/bugwatch/url" |
263 | - tal:content="comment/bugwatch/title" />, |
264 | + <a itemprop="url" |
265 | + tal:attributes="href comment/bugwatch/url" |
266 | + tal:content="comment/bugwatch/title" />, |
267 | <tal:comment_owner |
268 | replace="structure comment/owner/fmt:link-display-name-id" /> |
269 | wrote |
270 | - <span tal:attributes="title comment/datecreated/fmt:datetime" |
271 | - tal:content="comment/datecreated/fmt:displaydate"> |
272 | + <time |
273 | + itemprop="commentTime" |
274 | + tal:attributes="title comment/datecreated/fmt:datetime; |
275 | + datetime comment/datecreated/fmt:isodate" |
276 | + tal:content="comment/datecreated/fmt:displaydate"> |
277 | 7 minutes ago |
278 | - </span>: |
279 | + </time>: |
280 | <a tal:attributes="href comment/fmt:url"> |
281 | <strong tal:condition="comment/display_title" |
282 | tal:content="comment/title" /> |
283 | </a> |
284 | </td> |
285 | + |
286 | + <td> |
287 | + <tal:plusone |
288 | + metal:use-macro="context/@@+base-layout-macros/comment-plusone-button" /> |
289 | + </td> |
290 | + |
291 | <td class="bug-comment-index"> |
292 | - <a itemprop="url" |
293 | - tal:attributes="href comment/fmt:url" |
294 | + <a itemprop="url" |
295 | + tal:attributes="href comment/fmt:url" |
296 | tal:content="string: #${comment/index}" /> |
297 | </td> |
298 | </tr> |
299 | @@ -94,6 +104,7 @@ |
300 | <a tal:attributes="href comment/fmt:url">Read more...</a> |
301 | </p> |
302 | </div> |
303 | + |
304 | <div class="boardCommentFooter" tal:condition="comment/show_footer"> |
305 | <a tal:attributes="id string:mark-spam-${context/index};" |
306 | tal:condition="view/show_spam_controls" |
307 | |
308 | === modified file 'lib/lp/bugs/templates/bugcomment-index.pt' |
309 | --- lib/lp/bugs/templates/bugcomment-index.pt 2011-08-03 15:27:05 +0000 |
310 | +++ lib/lp/bugs/templates/bugcomment-index.pt 2011-11-27 22:43:42 +0000 |
311 | @@ -9,7 +9,7 @@ |
312 | <body> |
313 | <metal:block fill-slot="head_epilogue"> |
314 | </metal:block> |
315 | - <div metal:fill-slot="main" tal:define="comment view/comment"> |
316 | + <div metal:fill-slot="main" itemprop="mainContentOfPage" tal:define="comment view/comment"> |
317 | <h1 tal:content="view/page_title">Foo doesn't work</h1> |
318 | <tal:comment replace="structure comment/@@+box-expanded-reply"> |
319 | </tal:comment> |
320 | |
321 | === modified file 'lib/lp/bugs/templates/bugtask-index.pt' |
322 | --- lib/lp/bugs/templates/bugtask-index.pt 2011-11-24 05:14:44 +0000 |
323 | +++ lib/lp/bugs/templates/bugtask-index.pt 2011-11-27 22:43:42 +0000 |
324 | @@ -80,8 +80,13 @@ |
325 | tal:content="context/bug/datecreated/fmt:displaydate" /> |
326 | </tal:registering> |
327 | |
328 | - <metal:heading fill-slot="heading" tal:define="context_menu context/menu:context"> |
329 | - <h1 tal:replace="structure view/bug_title_edit_widget"> |
330 | + <metal:heading fill-slot="heading" |
331 | + tal:define="context_menu context/menu:context"> |
332 | + <tal:plusone |
333 | + metal:use-macro="context/@@+base-layout-macros/header-plusone-button" /> |
334 | + <h1 |
335 | + itemprop="name" |
336 | + tal:replace="structure view/bug_title_edit_widget"> |
337 | Bug title |
338 | </h1> |
339 | </metal:heading> |
340 | |
341 | === modified file 'lib/lp/code/browser/branchmergeproposal.py' |
342 | --- lib/lp/code/browser/branchmergeproposal.py 2011-11-20 05:56:28 +0000 |
343 | +++ lib/lp/code/browser/branchmergeproposal.py 2011-11-27 22:43:42 +0000 |
344 | @@ -594,7 +594,6 @@ |
345 | |
346 | implements(IBranchMergeProposalActionMenu) |
347 | |
348 | - label = "Proposal to merge branch" |
349 | schema = ClaimButton |
350 | |
351 | def initialize(self): |
352 | @@ -677,6 +676,12 @@ |
353 | return result |
354 | |
355 | @property |
356 | + def label(self): |
357 | + return "Merge %s into %s" % ( |
358 | + self.context.source_branch.bzr_identity, |
359 | + self.context.target_branch.bzr_identity) |
360 | + |
361 | + @property |
362 | def pending_diff(self): |
363 | return ( |
364 | self.context.next_preview_diff_job is not None or |
365 | |
366 | === modified file 'lib/lp/code/stories/branches/xx-code-review-comments.txt' |
367 | --- lib/lp/code/stories/branches/xx-code-review-comments.txt 2011-10-20 00:53:01 +0000 |
368 | +++ lib/lp/code/stories/branches/xx-code-review-comments.txt 2011-11-27 22:43:42 +0000 |
369 | @@ -112,8 +112,8 @@ |
370 | source branch to land it. When this comment is replied to, it should |
371 | wrap the line properly. |
372 | >>> print_comments('boardCommentDetails', anon_browser, index=0) |
373 | - Eric the Viking (eric) wrote ... ago |
374 | - Posted in a previous version of this proposal |
375 | + Eric the Viking (eric) wrote ... ago: |
376 | + Posted in a previous version of this proposal # |
377 | >>> details = find_tags_by_class( |
378 | ... anon_browser.contents, 'boardCommentDetails')[0] |
379 | >>> links = details.findAll('a') |
380 | @@ -133,7 +133,7 @@ |
381 | >>> eric_browser.getControl('Save Comment').click() |
382 | |
383 | >>> print_comments('boardCommentDetails', eric_browser, index=2) |
384 | - Eric the Viking ... a moment ago |
385 | + Eric the Viking ... ago: # |
386 | >>> print_comments('boardCommentFooter', eric_browser, index=0) |
387 | review: Abstain (timeless) |
388 | >>> print_comments('boardCommentBody', eric_browser, index=2) |
389 | |
390 | === modified file 'lib/lp/code/templates/branchmergeproposal-index.pt' |
391 | --- lib/lp/code/templates/branchmergeproposal-index.pt 2011-11-14 20:24:46 +0000 |
392 | +++ lib/lp/code/templates/branchmergeproposal-index.pt 2011-11-27 22:43:42 +0000 |
393 | @@ -50,6 +50,12 @@ |
394 | </style> |
395 | </metal:block> |
396 | |
397 | +<metal:block |
398 | + metal:fill-slot="heading"> |
399 | + <tal:plusone metal:use-macro="context/@@+base-layout-macros/header-plusone-button" /> |
400 | + <h1 itemprop="name" tal:content="view/label"></h1> |
401 | +</metal:block> |
402 | + |
403 | <tal:registering metal:fill-slot="registering"> |
404 | Proposed by |
405 | <tal:registrant replace="structure context/registrant/fmt:link"/> |
406 | |
407 | === modified file 'lib/lp/code/templates/codereviewcomment-body.pt' |
408 | --- lib/lp/code/templates/codereviewcomment-body.pt 2010-09-20 10:48:52 +0000 |
409 | +++ lib/lp/code/templates/codereviewcomment-body.pt 2011-11-27 22:43:42 +0000 |
410 | @@ -12,10 +12,12 @@ |
411 | </div> |
412 | </tal:good-attachments> |
413 | |
414 | - <div class="replyLink"> |
415 | - <tal:reply tal:define="link context/menu:context/reply" |
416 | - tal:condition="link/enabled" |
417 | - tal:replace="structure link/render"> |
418 | + <div class="replyLink" itemprop="replyToUrl" |
419 | + tal:define="link context/menu:context/reply" |
420 | + tal:condition="link/enabled" |
421 | + tal:attributes="url link/fmt:url"> |
422 | + <tal:reply |
423 | + tal:replace="structure link/render"> |
424 | Reply |
425 | </tal:reply> |
426 | </div> |
427 | |
428 | === modified file 'lib/lp/code/templates/codereviewcomment-header.pt' |
429 | --- lib/lp/code/templates/codereviewcomment-header.pt 2011-05-31 15:15:47 +0000 |
430 | +++ lib/lp/code/templates/codereviewcomment-header.pt 2011-11-27 22:43:42 +0000 |
431 | @@ -3,12 +3,35 @@ |
432 | xmlns:metal="http://xml.zope.org/namespaces/metal" |
433 | omit-tag=""> |
434 | |
435 | - <tal:author replace="structure context/comment_author/fmt:link-display-name-id"/> |
436 | +<table><tbody><tr> |
437 | + <td> |
438 | + <span |
439 | + itemprop="creator" |
440 | + tal:content="structure context/comment_author/fmt:link-display-name-id"/> |
441 | <tal:has-body condition="context/has_body">wrote</tal:has-body> |
442 | - <tal:date replace="context/comment_date/fmt:displaydate" /> |
443 | + <time |
444 | + itemprop="commentTime" |
445 | + tal:attributes="title context/comment_date/fmt:datetime; |
446 | + datetime context/comment_date/fmt:isodate" |
447 | + tal:content="context/comment_date/fmt:displaydate"> |
448 | + 7 minutes ago |
449 | + </time>: |
450 | <span tal:condition="context/from_superseded" class="sprite warning-icon" |
451 | style="float: right">Posted in <a |
452 | tal:attributes="href context/branch_merge_proposal/fmt:url">a |
453 | previous version</a> |
454 | of this proposal</span> |
455 | + </td> |
456 | + |
457 | + <td> |
458 | + <tal:plusone |
459 | + metal:use-macro="context/@@+base-layout-macros/comment-plusone-button" /> |
460 | + </td> |
461 | + |
462 | + <td class="bug-comment-index"> |
463 | + <a itemprop="url" |
464 | + tal:attributes="href context/fmt:url">#</a> |
465 | + </td> |
466 | + |
467 | +</tr></tbody></table> |
468 | </tal:root> |
469 | |
470 | === modified file 'lib/lp/registry/templates/distribution-index.pt' |
471 | --- lib/lp/registry/templates/distribution-index.pt 2011-06-16 13:50:58 +0000 |
472 | +++ lib/lp/registry/templates/distribution-index.pt 2011-11-27 22:43:42 +0000 |
473 | @@ -24,7 +24,11 @@ |
474 | |
475 | <body> |
476 | <tal:heading metal:fill-slot="heading"> |
477 | - <h1 tal:content="context/title">project title</h1> |
478 | + <div style="float:right"> |
479 | + <tal:plusone |
480 | + metal:use-macro="context/@@+base-layout-macros/header-plusone-button" /> |
481 | + </div> |
482 | + <h1 itemprop="name" tal:content="context/title">project title</h1> |
483 | </tal:heading> |
484 | |
485 | <tal:registering metal:fill-slot="registering"> |
486 | @@ -57,7 +61,7 @@ |
487 | </div> |
488 | |
489 | <div class="yui-g"> |
490 | - <div class="yui-u first"> |
491 | + <div itemprop="description" class="yui-u first"> |
492 | <tal:distro-information content="structure context/@@+details" /> |
493 | </div> |
494 | |
495 | |
496 | === modified file 'lib/lp/registry/templates/product-index.pt' |
497 | --- lib/lp/registry/templates/product-index.pt 2011-11-20 13:16:14 +0000 |
498 | +++ lib/lp/registry/templates/product-index.pt 2011-11-27 22:43:42 +0000 |
499 | @@ -48,6 +48,8 @@ |
500 | <body> |
501 | <tal:main metal:fill-slot="main" |
502 | define="overview_menu context/menu:overview"> |
503 | + <tal:plusone |
504 | + metal:use-macro="context/@@+base-layout-macros/header-plusone-button" /> |
505 | <div class="top-portlet"> |
506 | <p id="project-inactive" class="warning message" |
507 | tal:condition="not: context/active"> |
508 | @@ -55,8 +57,8 @@ |
509 | <a tal:replace="structure overview_menu/review_license/fmt:icon"/> |
510 | </p> |
511 | |
512 | - <div class="summary" |
513 | - tal:content="structure context/summary/fmt:markdown"> |
514 | + <div class="summary" |
515 | + tal:content="structure context/summary/fmt:markdown"> |
516 | $Product.summary goes here. This should be quite short, |
517 | just a single paragraph of text really, giving the project |
518 | highlights. |
519 | |
520 | === modified file 'lib/lp/services/comments/templates/comment.pt' |
521 | --- lib/lp/services/comments/templates/comment.pt 2010-02-19 19:16:32 +0000 |
522 | +++ lib/lp/services/comments/templates/comment.pt 2011-11-27 22:43:42 +0000 |
523 | @@ -3,12 +3,16 @@ |
524 | xmlns:metal="http://xml.zope.org/namespaces/metal" |
525 | omit-tag=""> |
526 | |
527 | - <div tal:attributes="class string:boardComment ${context/extra_css_class|nothing}"> |
528 | + <div |
529 | + itemscope="" |
530 | + itemtype="http://schema.org/UserComments" |
531 | + tal:attributes="class string:boardComment ${context/extra_css_class|nothing}"> |
532 | <div class="boardCommentDetails" |
533 | tal:content="structure context/@@+comment-header"> |
534 | Details - everyone has details. |
535 | </div> |
536 | <div class="boardCommentBody" |
537 | + itemprop="commentText" |
538 | tal:condition="context/has_body" |
539 | tal:content="structure context/@@+comment-body"> |
540 | The comment body |
541 | |
542 | === modified file 'lib/lp/services/features/flags.py' |
543 | --- lib/lp/services/features/flags.py 2011-11-25 12:34:41 +0000 |
544 | +++ lib/lp/services/features/flags.py 2011-11-27 22:43:42 +0000 |
545 | @@ -125,6 +125,12 @@ |
546 | 'enabled', |
547 | '', |
548 | ''), |
549 | + ('plusone.buttons.disabled', |
550 | + 'boolean', |
551 | + 'Disable Google plusone buttons.', |
552 | + 'enabled', |
553 | + '', |
554 | + ''), |
555 | ('profiling.enabled', |
556 | 'boolean', |
557 | 'Overrides config.profiling.profiling_allowed to permit profiling.', |
558 | |
559 | === modified file 'lib/lp/soyuz/templates/archive-index.pt' |
560 | --- lib/lp/soyuz/templates/archive-index.pt 2011-07-18 09:23:10 +0000 |
561 | +++ lib/lp/soyuz/templates/archive-index.pt 2011-11-27 22:43:42 +0000 |
562 | @@ -8,11 +8,20 @@ |
563 | > |
564 | <body> |
565 | <tal:heading metal:fill-slot="heading"> |
566 | - <h1 tal:condition="context/enabled" |
567 | + <div style="float:right" |
568 | + tal:condition="not:context/private" > |
569 | + <tal:plusone |
570 | + metal:use-macro="context/@@+base-layout-macros/header-plusone-button" /> |
571 | + </div> |
572 | + <meta itemprop="name" |
573 | + tal:attributes="content context/displayname"> |
574 | + <h1 |
575 | + tal:condition="context/enabled" |
576 | tal:replace="structure view/displayname_edit_widget"> |
577 | PPA for user |
578 | </h1> |
579 | - <h1 tal:condition="not: context/enabled" |
580 | + <h1 itemprop="name" |
581 | + tal:condition="not: context/enabled" |
582 | tal:content="context/displayname" |
583 | class="disabled"> |
584 | PPA for user |
Comments on BMPs do have URLs, we just don't expose them much. Mainly we expose the reply link.
Comment: /code.launchpad .net/~abentley/ launchpad/ person- bug-listings/ +merge/ 83418/comments/ 180265
https:/
Reply link: /code.launchpad .net/~abentley/ launchpad/ person- bug-listings/ +merge/ 83418/comments/ 180265/ +reply
https:/