mozilla :: #screenshots

17 Jul 2017
10:10GitHub[screenshots] johngruen commented on issue #3125: @SoftVision-PaulOiegas i figured out this issue, it appears the root el SVG el needs an explicit width and height declaration, _and_ `startBackground.js` needs a reference to the starred svg as well
10:12GitHub[screenshots] johngruen created fix-starred-icon (+1 new commit):
10:12GitHubscreenshots/fix-starred-icon 88dbe2e John Gruen: set dimensions for icon and add to startup
10:13GitHub[screenshots] johngruen closed pull request #3118: shot preview animation changes (master...preview-animations)
10:13GitHub[screenshots] johngruen pushed 1 new commit to master:
10:13GitHubscreenshots/master 81bc2cb John Gruen: Merge pull request #3118 from mozilla-services/preview-animations...
10:14GitHub[screenshots] johngruen opened pull request #3136: set dimensions for icon and add to startup (master...fix-starred-icon)
10:26cloudops-ansiblescreenshots-dev build #199: building mozilla/screenshots:latest
10:29cloudops-ansiblescreenshots-dev build #199: mozilla/screenshots:latest deployed to Dev
10:30GitHub[screenshots] johngruen opened issue #3137: Update full page save button to match selection save button
12:01GitHub[screenshots] SXZ1 commented on issue #3038: Nightly 16-07-17, Screenshots 10.5.0 with #3084 landed, I can still reproduce the bug from the first post, same STR: Screenshot of the browser console:...
12:04GitHub[screenshots] SXZ1 commented on issue #2895: Nightly 16-07-17, SC10.5.0, I can still reproduce bug from this comment:...
12:06GitHub[screenshots] SXZ1 commented on issue #2895: Browser console:...
12:16GitHub[screenshots] SXZ1 commented on issue #3038: Nightly 16-07-17, Screenshots 10.5.0 with #3084 landed, I can still reproduce the bug from the first post, same STR. Screenshot of the browser console:...
13:53GitHub[screenshots] niharikak101 created save-icon-fix (+1 new commit):
13:53GitHubscreenshots/save-icon-fix 7dbcc9b Niharika Khanna: update shot preview save icon
13:55GitHub[screenshots] niharikak101 opened pull request #3138: update shot preview save icon (
14:04circleci-botFailed: niharikak101's build (#2064; push) in mozilla-services/screenshots (save-icon-fix) --
15:00circleci-botFailed: niharikak101's build (#2065; retry by niharikak101) in mozilla-services/screenshots (save-icon-fix) --
15:23GitHub[screenshots] wresuolc pushed 1 new commit to master:
15:23GitHubscreenshots/master 59bc5ba Bizarro @clouserw (minimal permissions): remove misleading word
15:30_6a68looking really choppy. going to try restarting vidyo
15:36cloudops-ansiblescreenshots-dev build #200: building mozilla/screenshots:latest
15:37clouserwmiles: do you have everything you need for the node upgrade?
15:38cloudops-ansiblescreenshots-dev build #200: mozilla/screenshots:latest deployed to Dev
15:44milesclouserw: relud is taking care of it
15:47clouserwok, just wanted to make sure we weren't blocking
15:49_6a68niharika: hey! how's it going
15:50niharikaI can't seem to connect to the bugzilla server for some reason
15:51niharikaokay no, done!
15:51niharikaall good here
15:54_6a68niharika: cool. let me know when you get your account set up, and I"ll CC you on that bug
15:54niharikaokay :)
16:55_6a68jgruen: for this svg bug, how did you repro the missing case? want to be sure it's for reals working on my machine
16:55_6a68jgruen: also, what's the story with 10.6 not getting uplifted?
16:56_6a68is 10.5 already in beta? do we want to ship 10.5 without 10.6?
16:56* _6a68 looks at changelog
16:59_6a68answering my own question, 10.5 looks ok on its own (private browsing + fix shutdown crash)
17:02fzzzyis there a bug for removing the rating feature before rewriting the whole addon? I think so
17:02fzzzywhoops wrong channel
17:12557A743LW[screenshots] flodolo commented on pull request #3068 e97dcb1: We can't expose strings like this, we need proper plurals for these strings
17:12203A9W2WV[screenshots] flodolo commented on pull request #3068 e97dcb1: ```...
17:17GitHub[screenshots] flodolo commented on pull request #3068 e97dcb1: What "leave" means in this context? Log out? Might be worth a comment
17:18GitHub[screenshots] flodolo commented on pull request #3068 e97dcb1: We can't expose strings like this, we need proper plurals for these strings
17:36GitHub[screenshots] 6a68 closed issue #3125: A blank space is displayed instead of the "Screenshots" toolbar button
17:36GitHub[screenshots] 6a68 deleted fix-starred-icon at 88dbe2e:
17:48cloudops-ansiblescreenshots-dev build #201: building mozilla/screenshots:latest
17:51cloudops-ansiblescreenshots-dev build #201: mozilla/screenshots:latest deployed to Dev
17:53niharikaclouserw: Are there any docs on outgoing.m.o?
17:57clouserwniharika: let's join #amo
17:59niharika then
18:00clouserwyeah, I'm asking if we can just use theirs or if we should set up our own
18:00clouserwwhy does screenshots need a redirector?
18:01niharikaon the shot page to redirect to where the shot was taken
18:03clouserwbasically you just have a shared key with this outgoing service and send a hash in the URL
18:03clouserwand the hash has to match or it doesn't redirect
18:03clouserwrelud: hi. we need to pass our redirect URLs though something like
18:04clouserwsounds like AMO already has one they use. I'm not sure if we're sharing services like that or spinning up our own
18:04clouserwprobably our own? is that something you can help us with?
18:05reludi can ask jason about how amo does it, and we might be able to do the same
18:06clouserwcool. looks like oremj made most of the commits on that repo, if that matters
18:06clouserwthanks for the help
18:15niharikaclouserw: What do you think about adding a pop up that asks for confirmation before redirecting?
18:19clouserwas long as we can redirect safely we shouldn't need it
18:19clouserwdoes it open in a new tab?
18:21GitHub[screenshots] niharikak101 force-pushed shot-annotations from f628bee to e742138:
18:21GitHubscreenshots/shot-annotations e742138 Niharika Khanna: highlighter tool ui
18:22reludclouserw: jason says we can re-use outgoing.{stage,prod}
18:22reludi just need to configure it
18:24niharikait doesn't open in a new tab
18:25circleci-botFailed: niharikak101's build (#2068; push) in mozilla-services/screenshots (shot-annotations) --
18:36clouserwrelud: thanks!
18:36clouserwniharika: is AMO's outgoing URL function
18:37clouserwwe should probably have a redirect_url_allow_list also, and just put on it for now
18:38clouserw is the AMO test
18:40niharikathanks! I'll have a look at these
18:43_6a68niharika: ping me if you have any questions :-)
18:44niharikaI will!
18:51GitHub[screenshots] mozilla-pontoon pushed 1 new commit to master:
18:51GitHubscreenshots/master d9372b8 Mikalai Udodau: Pontoon: Update Belarusian (be) localization of Firefox Screenshots...
19:05cloudops-ansiblescreenshots-dev build #202: building mozilla/screenshots:latest
19:07cloudops-ansiblescreenshots-dev build #202: mozilla/screenshots:latest deployed to Dev
19:08GitHub[screenshots] johngruen commented on issue #3056: This got uplifted to 10.5
19:21GitHub[screenshots] johngruen created update-save-icon (+1 new commit):
19:21GitHubscreenshots/update-save-icon 9606d13 John Gruen: update on full page save to cloud
19:21GitHub[screenshots] johngruen opened pull request #3139: update on full page save to cloud (master...update-save-icon)
19:22GitHub[screenshots] johngruen force-pushed update-save-icon from 9606d13 to cd6d9cb:
19:22GitHubscreenshots/update-save-icon cd6d9cb John Gruen: update on full page save to cloud
19:24ulfrniharika: in bug 1381189, what do you mean by "load user-submittted domains" ?
19:24firebotBug is not accessible
19:29GitHub[screenshots] 6a68 commented on pull request #3068 e97dcb1: Thanks, @flodolo!...
19:58_6a68ulfr: on the shot page, there's a link to the top-level domain from which the shot was taken
19:59_6a68for example, see the BMO link on this shot page
19:59ulfrwhat I don't get is why that's a redirect, and not just a href
20:00_6a68hmm, good point. I think the redirect endpoint was supposed to help with metrics gathering, but I'm not sure. let me check
20:00_6a68that's actually the simplest answer here, I'd say :-P
20:01ulfrbut that page already has google analytics
20:02_6a68ulfr: yeah, the metrics thing was an optional 'from' parameter. I don't know why we have that redirect endpoint
20:02_6a68ian woudl know, but he's out till thursday
20:03_6a68I can dig around a little bit and see what can be uncovered
20:03ulfrthat'd be great. I'd rather not have to deal with an open redirect if we don't need it in the first place :)
20:04_6a68yeah, me too!
20:06_6a68ulfr: aha, looks like the original intent was to hide the URL of the referrer shot page
20:06_6a68I guess hide the existence of the shot from the website
20:06ulfrfrom the website the shot was taken from?
20:06ulfrthat.... ok
20:06_6a68Originally, the link went directly to the page, not to the TLD
20:07ulfryeah, but the link was generated *by* the site
20:07ulfrso I'm not sure what we're protecting against
20:07_6a68right, I'm grasping at straws here, as the original bug has a one-liner description
20:07ulfrthanks for digging it up, I'll comment on the issue in github
20:07_6a68I guess protecting the user from revealing their usage of screenshots to the site that was screenshotted?
20:08_6a68clouserw: does it make sense to you that we use the /redirect endpoint to try to hide the shot page from the screenshotted website?
20:09Gijscould just set rel=noreferrer on the link, right?
20:09Gijsor whatever the magic annotation is
20:09GijsIIRC there's now a meta tag as well
20:09GitHub[screenshots] jvehent reopened issue #740: Check how referrer information is sent
20:09_6a68Thanks for the suggestion! The original bug mentioned that IE doesn't support that tag
20:10Gijsbut it's a Firefox feature, why does IE matter?
20:10_6a68But that was, I think, back when page shot was potentially a cross-browser addon thing, and not a firefox thing
20:10_6a68yeah, this bug is ancient
20:10_6a68I guess IE technically matters because the shot page itself is viewable in any browser...but again, I'm failing to see what information is leaked in such a case
20:15_6a68ulfr: Oh, I think the issue is that shots are publicly viewable if you can figure out the URL. So exposing the shot URL is sharing the user's shot with the website
20:17ulfrbut, again, that shot came *from* the website
20:18_6a68Aha. So you're saying that, since the site has X information about you in the first place, you risk nothing by sharing some subset of that information via a screenshot
20:18_6a68That makes sense to me
20:18ulfrif wants to read your secret documents, they don't need to harvest your shots urls, they can jsut... read your secret documents!
20:19_6a68very good point, thanks ulfr
20:21_6a68clouserw: It seems like we use a redirect endpoint to hide the screenshot's URL from the website that was screenshotted. But, as pointed out in scrollback, if that site wants to do evil with your information, they already have it. So I guess we'll replace the redirect endpoint with a simple href link. Does this seem reasonable?
20:21_6a68just looking for a sanity check / second opinion
20:31GitHub[screenshots] mozilla-pontoon pushed 1 new commit to master:
20:31GitHubscreenshots/master 130a0b5 Artem Polivanchuk: Pontoon: Update Ukrainian (uk) localization of Firefox Screenshots...
20:36clouserw_6a68: I don't know of a reason not to (lets add the nohref stuff that we can)
20:43clouserwif we're getting rid of the redirect let's tell relud so he doesn't spend time setting one up
20:44_6a68oh yeah, good call
20:45cloudops-ansiblescreenshots-dev build #203: building mozilla/screenshots:latest
20:45_6a68clouserw: I don't seem to have GA access, do we have any metrics around outgoing URL clicks? fzzzy thought that was one of the reasons for the /redirect endpoint
20:46_6a68I think we can fix that inside an anchor tag, too, since we have GA in the page:
20:48cloudops-ansiblescreenshots-dev build #203: mozilla/screenshots:latest deployed to Dev
20:48GitHub[screenshots] johngruen commented on issue #2887: I think we're now good here
20:48GitHub[screenshots] johngruen closed issue #2887: Dark Theme Add-on Icons
20:49fzzzy_6a68: i don't think it actually got implemented but we pondered showing view counts/click counts as a feature
20:50_6a68makes sense, thanks
20:50GitHub[screenshots] johngruen created more-ux-nits (+1 new commit):
20:50GitHubscreenshots/more-ux-nits 8ab131b John Gruen: fix some ui nits
20:51GitHub[screenshots] johngruen commented on issue #2347: This is fixed in nightly
20:51GitHub[screenshots] johngruen closed issue #2347: Page Shot icon from the browser toolbar, is hardly visible on darker themes
20:52GitHub[screenshots] johngruen opened pull request #3140: fix some ui nits (master...more-ux-nits)
21:04reludclouserw: the amo outgoing setup uses this code:
21:04reludlmk when you have a setting for it, and i can set it up on my end
21:07clouserwrelud: got it. it sounds like we might be able to drop the redirect from the backlog ^
21:18reludyeah, just copy-pasting what i already had. i won't do any further work unless someone asks me to configure it.
21:20* ulfr prefers to remove features than added new ones
21:39GitHub[screenshots] johngruen created smooth-out-delete (+1 new commit):
21:39GitHubscreenshots/smooth-out-delete d604ec9 John Gruen: animate deleting shots on my shots
21:39GitHub[screenshots] johngruen opened pull request #3141: animate deleting shots on my shots (master...smooth-out-delete)
21:43GitHub[screenshots] johngruen force-pushed more-ux-nits from 8ab131b to df87c07:
21:43GitHubscreenshots/more-ux-nits df87c07 John Gruen: fix some ui nits
21:46GitHub[screenshots] johngruen commented on issue #2628: I'll do this once initial l10n is landed
21:47jgruen_6a68: spent some time this evening knocking out little UX PRs
21:47jgruenshould be low risk to merge all three
21:47jgruenhow is l10n going?
21:52circleci-botFixed: johngruen's build (#2075; push) in mozilla-services/screenshots (more-ux-nits) --
21:55_6a68jgruen: l10n update
22:39GitHub[screenshots] 6a68 opened issue #3142: Merge back divergent changes in addon release branch
22:46GitHub[screenshots] 6a68 created 10.7-changes (+2 new commits):
22:46GitHubscreenshots/10.7-changes 2569967 John Gruen: set dimensions for icon and add to startup (#3136)
22:46GitHubscreenshots/10.7-changes ba747a5 Jared Hirsch: Address 10.6 review comments (bug 1381132)
22:46firebot NEW, Update Screenshots to version 10.6.0
22:50GitHub[screenshots] 6a68 opened pull request #3143: 10.7 changes (v10.4.0-export...10.7-changes)
23:00GitHub[screenshots] 6a68 commented on issue #3143: Seems to be working for me, moving right along
23:02GitHub[screenshots] 6a68 force-pushed 10.7-changes from ba747a5 to f0aa032:
23:02GitHubscreenshots/10.7-changes f0aa032 Jared Hirsch: Address 10.6 review comments (bug 1381132)
23:02firebot NEW, Update Screenshots to version 10.6.0
23:08GitHub[screenshots] 6a68 commented on pull request #3068 e97dcb1: @flodolo hmm, if stas is out of office, I'm inclined to land this with the existing pluralization and file a bug to fix later. 55 goes to release channel August 8th, so we have less than a month to localize the server strings. Are you OK with fixing the plurals later?
23:24GitHub[screenshots] 6a68 tagged 10.7.0 at ff2d8a4:
23:24GitHubscreenshots/10.7.0 ff2d8a4 Jared Hirsch: Update version to 10.7.0 with changelog
23:25GitHub[screenshots] 6a68 deleted 10.7.0 at ff2d8a4:
23:26GitHub[screenshots] 6a68 pushed 1 new commit to 10.7-changes:
23:26GitHubscreenshots/10.7-changes ff2d8a4 Jared Hirsch: Update version to 10.7.0 with changelog
23:27GitHub[screenshots] 6a68 pushed 1 new commit to v10.4.0-export:
23:27GitHubscreenshots/v10.4.0-export b469873 Jared Hirsch: Update addon export branch to 10.7.0 (#3143)...
23:27GitHub[screenshots] 6a68 deleted 10.7-changes at ff2d8a4:
23:28GitHub[screenshots] 6a68 tagged 10.7.0 at v10.4.0-export:
18 Jul 2017
No messages
Last message: 67 days and 4 hours ago