mozilla :: #screenshots

12 Jul 2017
06:13GitHub[screenshots] youwenliang commented on issue #3065: I think the original design is to click "Get started" and it will scroll down to "How Firefox screenshots works" section.
07:58GitHub[screenshots] niharikak101 commented on pull request #3082 b750905: We have a couple when the user clicks on disconnect, cancels or confirms, so we could skip this I think. @ianb
08:14GitHub[screenshots] niharikak101 force-pushed fxa from b750905 to 9c6451e:
08:14GitHubscreenshots/fxa 9c6451e Niharika Khanna: let user modify shot info
08:23circleci-botFailed: niharikak101's build (#1982; push) in mozilla-services/screenshots (fxa) --
08:23GitHub[screenshots] niharikak101 force-pushed fxa from 9c6451e to 52ea9f2:
08:23GitHubscreenshots/fxa 52ea9f2 Niharika Khanna: let user modify shot info
08:32GitHub[screenshots] niharikak101 force-pushed fxa from 52ea9f2 to 2cc40d5:
08:32GitHubscreenshots/fxa 2cc40d5 Niharika Khanna: let user modify shot info
08:33circleci-botFailed: niharikak101's build (#1983; push) in mozilla-services/screenshots (fxa) --
09:08GitHub[screenshots] youwenliang opened issue #3112: Annotate Screenshots
09:18GitHub[screenshots] emilghittasv commented on issue #3097: I just want to add that I encountered the "Shot is not defined" error while following mainly the up mentioned steps on the webpage....
09:19GitHub[screenshots] niharikak101 commented on issue #3112: @ianb and I talked about adding a highlighter tool. What do you think @youwenliang?
09:19GitHub[screenshots] niharikak101 force-pushed fxa from 2cc40d5 to a612ada:
09:19GitHubscreenshots/fxa a612ada Niharika Khanna: let user modify shot info
09:29circleci-botFailed: niharikak101's build (#1985; push) in mozilla-services/screenshots (fxa) --
10:48GitHub[screenshots] youwenliang commented on issue #3112: @niharikak101 Sure! I do have a highlighter tool in the previous version but somehow I dropped it, I can add it back!...
14:05GitHub[screenshots] tomrittervg commented on issue #2826: This is a (semi-)automated bug making you aware that there is an available upgrade for an embedded third-party library. You can leave this bug open, and it will be updated if a newer version of the library becomes available. If you close it as WONTFIX, please indicate if you do not wish to receive any future bugs upon new releases of the library....
14:41GitHub[screenshots] niharikak101 force-pushed fxa from a612ada to 8cd0772:
14:41GitHubscreenshots/fxa 8cd0772 Niharika Khanna: let user modify shot info
14:45jgruenianbicking: getting some animations ready to publish the grad report of Page Shot to our new blog
14:46jgruenianbicking: is 5.2 the last test pilot release?
15:16Gijs_awayinside the add-on, what's the sensible way to get the URL for the screenshots page that lists the user's screenshots?
15:17GijsI assume there's a pref of some description?
15:21jgruenGijs: ^ hit that My Shots button
15:21Gijsjgruen: I'm being unclear
15:22Gijsjgruen: I'm trying to write code in the add-on
15:22Gijsjgruen: how do I reference that URL?
15:22Gijswhat, it's hardcoded to be just that?
15:22jgruenoh, no, internal to the add-on it's not
15:23Gijslooks like there's a 'backend' variable and a hardcoded 'shots' bit
15:24jgruenGijs: yeah looks like it, are you asking WRT the library implementation?
15:25jgrueni'm sure we could find a better solution for you. I'll bring it up in today's standup
15:25jgruendo you have a preferred method of getting that URL?
15:26Gijsnah, I think it's fine
15:26Gijsit looks like I could update the bootstrap.js listener to respond to a backend message
15:27Gijsand just get the URL that way
15:27Gijsthis assumes it won't change at runtime
15:27Gijswhich seems like a reasonable assumption?
15:27jgruenyes, considering it took considerable time to secure that URL
15:28ianbickingGijs: yes, we dont support configuring the backend variable. We actually pull it out of manifest.json in our own code
15:29Gijshrm, why did I not see it in manifest.json?
15:29GijsI did look there
15:29Gijswell, the template in the repo
15:29Gijsbut it didn't have any obvious variables that looked like the right thing
15:29ianbickingGijs: well, its just in the permissions key, as the only URL, which is admittedly very implicit
15:29Gijsuh, interesting
15:29GijsI could probably hack something on top of that, but that feels fragile
15:31ianbickingGijs: yeah, its not great, though theres not a lot of spots for declaring something in a WebExtension. Or maybe there is and I just dont know about it?
15:32GijsI have no idea :)
15:32* Gijs should be more into webextensions than he is right now, but hasn't had time (hi photon)
15:32Gijsnaively, I expect you could do whatever you like in webextension manifest keys
15:32Gijsor use the storage module or some other prefs-based state thing
15:33Gijsbut if it works, don't fix it :)
15:33ianbickingGijs: the storage stuff seems kind of opaque, I think unexpected keys in manifest.json lead to some warnings in the console, but maybe theres a space that doesnt do that
15:36Gijsheh, main.js breaks syntax highlighting for me
15:36Gijs backend = backend.replace(/\/*$/, "");
15:36Gijslooks like a comment start
15:36Gijshow would I test Firefox with a modified version of the add-on?
15:36GijsI guess I could just copy/paste my changes back into the main m-c tree?
15:36ianbickingGijs: ./bin/run-addon bootstrap
15:36Gijsoh, cool, thanks
15:42* Gijs wonders how to get an icon url...
15:45ianbickingGijs: to get an icon URL you need to know the moz-extension://UUID for the extension someone on #teamaddons will know (some of these discussions might better over there, where the right people might overhear us)
15:46GitHub[screenshots] johngruen closed issue #3100: Make icon work on dark/light themes
16:04_6a68jgruen | ianbicking: do we know the median screenshot image size?
16:05ianbicking_6a68: the stats are a little wonky, but more or less yes
16:05_6a68ianbicking: I couldnt' find it in data studio, where might that number be?
16:05ianbicking_6a68: tend to be wide, bifurcated between mediumish and small height
16:05_6a68I was thinking we could pick the average-ish image size, create a shot, and use that as a measurement baseline for the web perf OKR
16:06_6a68I just took what I thought was a small shot on my machine, turns out it's a 1 megabyte 1300x1200 image, which is likely not the typical size
16:07ianbicking_6a68: and
16:08ianbicking_6a68: having a good baseline image would be good for measurement, yeah; though I bet well be able to improve performance without changing the image itself
16:09_6a68I agree, adding a CDN would get us a long way with very little dev work
16:09niharikaianbicking: I've tested stuff and there seem to be no errors, but there's still a manage account button on the settings page that doesn't do anything. I think remove it for now if we want to land this soon? I can add it back as we add more features in the future
16:09ianbickingtechnically dev work is done on that
16:09ianbickingmoving scripts down in the body would help too
16:09_6a68is 1220 the median shot width because it's the median full window size? seems odd
16:10Gijsianbicking: so I'm trying the run-addon thing but it seems like it wants to go talk to the network and I'm in a hotel and my network is crap, so it broke and now it's perma-broke
16:10GijsError: Cannot find module 'raven-js/dist/raven.js'
16:10ianbickingniharika: sure, I dont know what that should do, except maybe link to ? Leaving it out is fine
16:11ianbickingGijs: theres a make rule that tries to find the location of raven.js from node_modules did npm install succeed?
16:11Gijsno, broken checksums because the downloads broke
16:11Gijshow do I remove the broken stuff so it tries again?
16:11Gijs(why doesn't npm just do that?)
16:11ianbickingGijs: npm install should probably get everything right?
16:12ianbickingbut I dont know the details of it, maybe you have to blow away node_modules
16:12GijsI have no idea
16:12niharikaYes, but we would want the changes made on to reflect back? I'll leave it out for now
16:12GijsI'm kinda hoping it's not fetching all the server stuff I don't care about
16:12Gijsthis network connection is bad enough as it is
16:12ianbickingniharika: true, since we dont update its not a great thing to push people towards
16:12ianbickingGijs: it gets both, yeah, sorry
16:18GitHub[screenshots] niharikak101 commented on issue #3112: @youwenliang I'll have to look more into the implementation, but I think it's definitely possible!
16:18Gijshm, still hitting EINTEGRITY check fails
16:25Gijsthe hashes in the EINTEGRITY checks keep changing
16:25GijsI wonder if that means it's slowly getting more stuff or if it's just perma-busted
16:26ianbickingits beyond my npm understanding Im afraid
16:32_6a68Gijs: maybe related to
16:33_6a68honestly I can't tell, that issue is a mile long and was auto-closed
16:33_6a68maybe just tether off your phone to run npm install :-(
16:34_6a68Gijs: try downgrading to npm 4?
16:35_6a68if that's possible on a hotel network ;-)
16:40Gijsprobably not a good idea
16:40GijsI got the code to run by just copying things
16:40ianbickingcool the extension build process doesnt do a ton of stuff
16:40Gijsbut I'm trying to get logging from the code and not succeeding - where do the log.error() things from the webextension show up? Do I need to flip a pref for that to work?
16:42ianbickingGijs: they should go right in the browser console
16:43ianbickingGijs: you can use about:debugging to get to the actual background page, where main.js is run, but a lot of it you can see with just the browser console (which is what I mostly use now)
16:43_6a68FWIW the log level is controlled by an env var at build time, but the default should show errors
16:44Gijshm, odd
16:45GijsI'll poke more at this after dinner, maybe :)
16:45Gijssilent failures are the worst...
16:49_6a68Gijs_away: an easy way to verify errors are making it to the browser console: open up about:config, then hit the screenshots button. You should see a "Missing host permission for the tab" error in the browser console
16:50ianbickingevery so often the console has failed on me, sometimes blowing away my profile helped
16:52Gijs_awaysyntax error in main.js
16:52Gijs_awaywhich somehow didn't get reported before doing that
16:52Gijs_away_6a68: thanks!
16:52Gijs_away(also not by ./mach lint :s )
16:52ianbickingGijs_away: oh, main.js is lazy-loaded, thats probably why
16:52_6a68ah yeah! we lazy load all the things
16:53_6a68or rather, we defer loading all the things till the button is clicked
16:57Gijs_awayhm, might need to rethink some of what I'm doing then...
16:57Gijs_awayanyway, dinner first
17:10GitHub[screenshots] niharikak101 force-pushed fxa from 8cd0772 to a29e106:
17:10GitHubscreenshots/fxa a29e106 Niharika Khanna: let user modify shot info
17:23GitHub[screenshots] niharikak101 force-pushed fxa from a29e106 to 4f7a2ac:
17:23GitHubscreenshots/fxa 4f7a2ac Niharika Khanna: let user modify shot info
17:24_6a68I dropped some hints in the bugzilla bug (1366026) for Gijs, since he bailed out of channel to eat
17:54jawspageshot is ending tomorrow?
17:57_6a68boy, npm 5 could use some QA help
17:57_6a68jaws: pageshot = , screenshots =
17:58jaws_6a68: yes, but is there going to be a gap for users?
17:58jawsweeks where users who were using pageshot will have to wait for screenshots?
17:58ianbickingjaws: PageShot stays installed until you activate Screenshots
17:58ianbickingwere not shutting down servers or anything
17:58jawsianbicking: even though says it ends tomorrow?
17:58ianbickingjaws: thats when it gets removed from the testpilot site
17:58_6a68I think you just won't be able to newly install it from the TxP website
17:59ianbickingtheres no magic self-destruct in Test Pilot, so ending is just about the promotional part
18:48GitHub[screenshots] ianb created deploy-to-stage (+1 new commit):
18:48GitHubscreenshots/deploy-to-stage 2dfacd3 Ian Bicking: Deploy to stage when merging to stable
18:49GitHub[screenshots] ianb opened pull request #3113: Deploy to stage when merging to stable (master...deploy-to-stage)
18:53GitHub[screenshots] 6a68 commented on issue #3068: Adding my current TODO list to try to give some visibility into progress:...
19:05GitHub[screenshots] 6a68 commented on issue #3068: current TODO list:...
19:06GitHub[screenshots] 6a68 commented on issue #3068: current TODO list:...
19:11GitHub[screenshots] 6a68 commented on issue #3068: current TODO list:...
19:14GitHub[screenshots] 6a68 commented on issue #3068: current TODO list:...
19:18GitHub[screenshots] 6a68 commented on issue #3068: current TODO list:...
19:29clouserwshield folks are prepping to put us up to 100% on beta (email going out to release-drivers soon)
20:03GitHub[screenshots] ianb closed issue #2951: 9.x.0 deployment notes
20:31clouserwianbicking: patch is r+'d!
20:32clouserwnow we flag it for needs landing?
20:44ianbickingclouserw: yep, flagged it
20:55GitHub[screenshots] 6a68 commented on issue #3068: current TODO list:...
20:56GitHub[screenshots] pauljt commented on issue #3098: As per Julian's comment - there has been a security review (see for issues raised). That said, if there are any additional risks please raise them (though is the preferred channel for private reporting once a product goes to release)
20:58GitHub[screenshots] pauljt commented on issue #3098: FWIW I dont _think_ scraping is a risk - when I looked at the earlier version of this product, the entropy of the uploaded screenshot URLs seemed pretty high, so guessing/bruteforcing seemed unlikely - but if this isnt true (and we are leaking the URIs somehow) this is something we'd want to know about.
21:07GitHub[screenshots] 6a68 commented on issue #3068: current TODO list:...
21:12GitHub[screenshots] pauljt commented on issue #3098: Also note that screenshots uses web server headers like x-frame-options to prevent things like click-jacking and a strict CSP. Coercing the user to screenshot data is certainly a risk, but using cross-origin framing isn't allowed. The attacker would need to coerce the user to a) take a screenshot of a sensitive document, AND then b) convince the users to disclose the URI (containing the UUID tok
21:14GitHub[screenshots] ianb commented on issue #3068: :+1: on followup for home page, react keys. Probably window.confirm too, we can just change it to "Delete this shot?" for now (or heck, forever).
22:19GitHub[screenshots] 6a68 commented on issue #3068: > Probably window.confirm too, we can just change it to "Delete this shot?" for now (or heck, forever)...
22:32GitHub[screenshots] 6a68 force-pushed localize-server from 59b6f10 to 3103a6d:
22:32GitHubscreenshots/localize-server 3103a6d Jared Hirsch: localize server...
22:33GitHub[screenshots] 6a68 commented on issue #3068: doing a final run-through now
22:40circleci-botFailed: 6a68's build (#1991; push) in mozilla-services/screenshots (localize-server) --
22:51GitHub[screenshots] 6a68 commented on issue #3068: did a final rebase and ran through all the flows/pages. everything seems to work :+1:...
22:59GitHub[screenshots] wresuolc commented on issue #3098: thanks for the report and replies...
23:01GitHub[screenshots] 6a68 force-pushed localize-server from 3103a6d to 5a3bd08:
23:01GitHubscreenshots/localize-server 5a3bd08 Jared Hirsch: localize server...
23:07GitHub[screenshots] jan-ivar commented on issue #3098: Glad to hear cross-origin framing isn't allowed!
23:07GitHub[screenshots] jan-ivar commented on issue #3098: Glad to hear cross-origin framing is disallowed!
23:11GitHub[screenshots] 6a68 opened issue #3114: Make homepage product tour headers bold
23:19GitHub[screenshots] 6a68 force-pushed localize-server from 5a3bd08 to e0cee52:
23:19GitHubscreenshots/localize-server e0cee52 Jared Hirsch: localize server...
23:27circleci-botFixed: 6a68's build (#1993; push) in mozilla-services/screenshots (localize-server) --
23:27GitHub[screenshots] 6a68 opened issue #3115: My Shots page: `key` prop warning on Card components
23:35GitHub[screenshots] 6a68 opened issue #3116: Make truncation RTL-friendly
23:36GitHub[screenshots] 6a68 force-pushed localize-server from e0cee52 to f5938db:
23:36GitHubscreenshots/localize-server f5938db Jared Hirsch: localize server...
23:37GitHub[screenshots] 6a68 opened issue #3117: Simplify l10n.js getText method
23:57GitHub[screenshots] 6a68 commented on issue #3068: @ianb OK, I think this is fully baked. Mind taking a last look before landing?
13 Jul 2017
No messages
Last message: 14 days and 23 hours ago