mozilla :: #mdndev

21 Apr 2017
00:10rjohnsongood night #mdndev
14:12jwhitlockmoing #mdndev
14:12jwhitlockthat's not even
14:12jwhitlockgood morning
14:48safwanGood evening #mdndev
14:48safwanjwhitlock: Hey, possible to have a review today?
14:52jwhitlocksafwan: PR 4158? sure.
14:52safwanjwhitlock: Yap
15:27sheppyOK, back to this thing... anyone who&#39;s on now that wasn&#39;t yesterday evening have thoughts on what would cause me to get &quot;18:00:39.603 Loading failed for the <script> with source http://localhost:8000/static/js/analytics.js. 1 HTML:917&quot;, but if I click on analytics.js on that line in view source, it loads fine?
15:50safwansheppy: Whats the problem?
15:52sheppysafwan: On my Docker setup here, I&#39;m trying to test some tweaks to a plugin for CKEditor, and after updating everything to the latest Kuma here, I&#39;m getting that error trying to load analytics.js on page load. It&#39;s killing scripts, leaving many UI elements not working.
15:54sheppyCan&#39;t figure out why it fails to load that file. It&#39;s there. Clicking it in the view-source for the page that says it can&#39;t load it loads it fine.
15:54stephend|commutesheppy: wouldn&#39;t be CSP or something, would it?
15:54safwansheppy: Can you try running &quot;docker-compose exec web python collectstatic&quot;
15:55sheppystephend|commute: Given that all the other JS that&#39;s loaded by the same page from the same directory works fine, that seems unlikely...
15:55safwansheppy: Have you turned on DNT?
15:56sheppyYeah, I have DNT on.
15:56* sheppy runs that collectstatic
15:57sheppyThat didn&#39;t change anything...
15:58sheppyWhat&#39;s weird is that there isn&#39;t even an entry in the Network tab of the dev tools for the attempt to load analytics.js.
15:58sheppyIt&#39;s like it never even tries.
15:58safwansheppy: You should turn off DNT and then try
15:59sheppyWhich is probably why the error isn&#39;t an HTTP error with a code.
15:59sheppyHuh. OK.
16:00sheppyNo change.
16:03sheppyAnd I can still click the link in the view source for the page and the analytics.js file loads right up.
16:04safwansheppy: analytics.js loads up?
16:05sheppysafwan: If I view-source the page, then click the link to analytics.js that appears in the source, yes.
16:05sheppyYet when the page tries to load it it can&#39;t.
16:05safwancan you give a screenshot of network tab?
16:05sheppyI don&#39;t know where the message &quot;Loading failed for the <script> with the source &quot;http://localhost:8000/static/js/analytics.js&quot; comes from.
16:05sheppysafwan: Sure, but analytics.js isn&#39;t listed there at all...
16:06safwansheppy: Where do you see the error? in console?
16:06sheppysafwan: yes
16:06safwangive a screenshot
16:18sheppyThis is so weird.
16:38sheppyAh, github. You fail me again.
16:38sheppyThis time disconnecting in the middle of a push. WTF?
16:41safwansheppy: I am trying to setup kuma, but failing due to it
16:44sheppyOK, time for me to get set to head to my appointment anyway. I&#39;ll be back later.
17:05github[kumascript] stephaniehobson created stephaniehobson-empty-embedcompattable (+1 new commit):
17:05githubkumascript/stephaniehobson-empty-embedcompattable e51dcd7 Stephanie Hobson: Empty EmbedCompatTable.ejs...
17:05github[kumascript] stephaniehobson opened pull request #161: Empty EmbedCompatTable.ejs (master...stephaniehobson-empty-embedcompattable)
20:42sheppyOK, I did like all this stuff to get up to date with my repo, yet looking at it on github, it says I&#39;m 150 commits behind mozilla:master. What. The. Hell?
20:43sheppyHrm. Somehow all the stuff I updated is only such on my branch for this work. That&#39;s annoying. OK.
20:44sheppyOh for crying out loud, there&#39;s a conflict that came to be during the time I was at my appointment. :)
20:47sheppyHum. How do you handle a conflict in which the only difference is lines 6-8 of locale/cs/LC_MESSAGES/django.po, being the creation date of the POT, the revision date of the PO, and the translator&#39;s ID.
20:47sheppyThere are no other differences, which is weird.
20:49sheppyOh, I think I guess what it is. Still...
20:49sheppyI think I will keep my dates but the other translator&#39;s name.
20:53jwhitlocksheppy: locale files changes should not be in your PR
20:53sheppyjwhitlock: The instructions say to add everything in locale.
20:53sheppySo I did.
20:54sheppygit add --all locale
20:54sheppyThat line is right there in the instructions, before the commit.
20:55jwhitlockyeah but you shouldn&#39;t mix code changes with locale updates
20:55sheppyBut that&#39;s the entire point of the change.
20:55sheppyThe code changes are required for the locale updates to work and vice versa.
20:55sheppyTests would fail otherwise, no?
20:56sheppyThese unstated rules are getting a little frustrating. :)
20:57* sheppy pauses work on trying to remember how to merge these two commits to deal with this information.
20:58jwhitlockok working on it
21:12sheppyThe speed you guys can do this with makes me feel sad inside. :)
21:13sheppyI keep working at it and am slowly getting better but I wish I&#39;d started longer ago instead of avoiding it as long as I could.
21:13sheppyI was all excited because things seemed to be going smoothly, until suddenly I hit the wall.
21:17jwhitlockok, a few things that went wrong
21:17jwhitlocksheppy: do you have time for a post-mortem, or do you need to move on?
21:17sheppyjwhitlock: I have time. This is important. It&#39;s how I&#39;ll stop needing help someday :)
21:18jwhitlockso, first thing was starting 150 commits behind master
21:18jwhitlockwas that because you started this work a while ago, or did you start recently?
21:18sheppyjwhitlock: It said I was up to date. I compared the log and everything.
21:18sheppyI started just yesterday afternoon.
21:18sheppyI made very sure to be up to date before I started to work.
21:19sheppyI don&#39;t know how it wound up not being. :(
21:19sheppyAlthough apparently I didn&#39;t do it until after branching, which I was pretty sure I didn&#39;t do.
21:20jwhitlockok, I show you branched at 8476a89, which was yesterday 2:51 PM my time
21:20jwhitlockso, I think you got that right
21:20sheppyI must have done the pull to update after the branch by mistake, which is weird because I know that&#39;s not right...
21:21jwhitlocknow, a2sheppy/master is at Marc 23rd, 2017, I can believe that&#39;s 150 commits behind
21:21jwhitlocksorry, March 23rd, not Marc 23
21:21sheppyYeah, master is behind somehow, which is how I gathered I got those operations backward.
21:21jwhitlockthat would the Toronto work week
21:22jwhitlocka2sheppy/master doesn&#39;t follow mozilla/master at all, unless you push to it
21:22jwhitlocka2sheppy/master is the master branch in your clone in github
21:23jwhitlockmozilla/master is the master branch in the mozilla repo on github
21:23jwhitlockand you have a branch called master on your local working copy
21:23jwhitlockand I have my own master branch etc.
21:23jwhitlocknone of these automatically sync unless someone does it
21:23sheppyOh. Yeah.
21:24sheppyI know that...
21:25jwhitlockso I think that&#39;s where the 150 commits behind came in
21:25jwhitlockbut you did the right thing - you started work on the mozilla/master branch like you were supposed to
21:25jwhitlockso that worked this time
21:26sheppyYeah, the branch was up to date at least.
21:26sheppyBut I need to do better about making sure I update my master
21:26sheppy(and I have an issue with that when we&#39;re done with post morten :)
21:27sheppymortem. Sheesh.
21:27jwhitlockhmm I have some additional changed files
21:28jwhitlockkuma/static/js/libs/ckeditor/build/ckeditor/ckeditor.js, ckeditor/skins/moono/editor.css, ckeditor/skins/moono/editor_gecko.css, etc.
21:29sheppyThose are from rebuilding CKEditor. I didn&#39;t add them to my commits because they&#39;re not supposed to be there, no?
21:29jwhitlockthere&#39;s an embedded timestamp that is updated with each build
21:29jwhitlockI hate it
21:30jwhitlockI&#39;m also considering comiting my copy because I got timestamp:&quot;H3LL&quot;
21:32jwhitlockok , the skins/moono/* ones appear to have the compiled mdn-sampler, so I&#39;m adding a commit
21:32sheppyjwhitlock: OK
21:33jwhitlockso, I think you went down the rabbit hole of updating locales because I said something about that
21:33jwhitlockwell, I&#39;m not sure why you were updating locales
21:34sheppyI mean, my change did involve updating locale files, because I changed some strings.
21:34sheppyWouldn&#39;t that mean locales would have to reflect the renamed strings?
21:35sheppyOr am I supposed to leave them broken and let the locale teams deal with it?
21:35jwhitlockyeah, but we don&#39;t include that in PRs
21:35jwhitlockyes, that&#39;s not documented
21:36sheppyNot sure what to make of this... how else does this happen?
21:36jwhitlockbut locales get updated several times a day, so including the locale updates in the PR is asking for merge conflicts
21:36sheppyOkay... so what was I supposed to do?
21:37sheppyHeh... next time someone says &quot;this wouldn&#39;t have happened if you&#39;d followed instructions&quot; I&#39;m going to laugh at them. :)
21:37jwhitlocklooking at the new diff in github, I don&#39;t think I should include those built files
21:38jwhitlockthe plugin is not in the source files, just the button, which didn&#39;t change
21:38sheppyOh. Yeah.
21:38sheppyI get you.
21:39jwhitlockok gone
21:39jwhitlockso, locale changes
21:39jwhitlockin the PRs we just change the english strings in the code
21:39jwhitlockso what it looks like now -
21:40jwhitlockjust before a production push, I peek at travis -
21:40sheppyAh, so you guys deal with it then.
21:41jwhitlockthat last build step builds &quot;locales&quot;
21:41sheppyProbably useful to mention this in the docs then, for the next guy like me to come along :)
21:41jwhitlockit&#39;s an allowed failure, because bad behavior from pontoon users can cause it to break
21:42jwhitlockat the very end, it extracts the locales, and then shows if there are any changed strings -
21:43jwhitlockwhen I see that output, I know I need to update the locale strings
21:43sheppyMan, I worked too hard on this PR. :)
21:43jwhitlockmy process is to do this, following the instructions in the kuma docs, and then commit the new local files directly to master
21:44jwhitlockat the same time, I update the kumascript submodule, also committing to master
21:44jwhitlockyes, it&#39;s not fully documented, because I&#39;m the only one doing it, and I&#39;m too lazy to document it
21:44jwhitlockthe next time I touch the documents, I want it to be when I remove Vagrant
21:45sheppyjwhitlock: At least mentioning that we shouldn&#39;t be doing it... :)
21:45* sheppy may do it then.
21:46jwhitlockhmm not working, maybe I do need those moono files
21:52jwhitlockok, here&#39;s here you asked about strings
21:53jwhitlockoh yeah while I was working w/ jswisher on the pontoon info
21:53jwhitlockso I should have dug deeper and asked what you were doing
21:54sheppyNo worries. :)
21:54jwhitlockI don&#39;t see you mentioning &quot;fix bug 1298615&quot; at any point though, so I thought maybe you were working on something to do with a different locale
21:55sheppyGood point. At the time I wasn&#39;t thinking of it in terms of a bug; I just happened to see the issue involved come up on me and it got on my very last nerve. :)
21:56jwhitlockso, to check your changes, I ran the steps under &quot;Building CKEditor&quot;:
21:56jwhitlockthese worked for me the very first time
21:56jwhitlockif they didn&#39;t work for you, that should have been a warning bell
21:56sheppyThey worked fine.
21:56jwhitlockwell you had some issue with analytics.js
21:57sheppyOh. Yeah. I think that&#39;s still happening to me actually.
21:57sheppyI have no idea why.
21:58sheppyDidn&#39;t think of that in the context of the CKEditor build...
21:58jwhitlockit&#39;s a good idea to start w/ a working dev environment before you start code changes
21:59sheppyWell yeah. I try to but after almost a full day of failing to get any insight into why it didn&#39;t work, and the changes being minor ones that could be tested despite the problem, I quit worrying about it for the duration.
22:00sheppyI did keep trying to find some assistance on it though.
22:00jwhitlocklooking at the screenshots, it didn&#39;t looks like analytics.js was a 404, it looked like an JS error
22:00sheppySpent a lot of time running through old lists of things I&#39;ve been helped with in the past.
22:00sheppyjwhitlock: Yes.
22:00sheppyWell, I don&#39;t know what it was. But no, not an HTTP error.
22:01jwhitlockI use the debugger or the console to diagnose JS errors
22:01sheppyYes, I did that too. And found nothing.
22:01* jwhitlock looks at screenshot again
22:02sheppyIt never even tries to do it, and it&#39;s an HTML thing, so the JS debugger is useless.
22:03sheppyThere&#39;s no attempt made to load it, so the network panel is useless.
22:03sheppyAll I know is that there&#39;s a failure to load it at line 917 of the HTML.
22:03sheppyBut why?
22:03jwhitlockok there it is
22:04jwhitlockso it is saying there is an error at line 917 of the HTML
22:04jwhitlockHTML changes on every page - what is line 917 of your HTML?
22:05sheppy<script type=&quot;text/javascript&quot; src=&quot;/static/js/analytics.js&quot; charset=&quot;utf-8&quot;></script>
22:05jwhitlockok, super accurate, huh
22:06sheppyI can&#39;t see any reason for the failure. That code is done exactly like all the ones around it, that path works when I click it in view source...
22:06sheppyOpens fine. So why is this error happening?
22:09jwhitlockcan you enable &quot;Pause On Exceptions&quot; in the gear menu in the debugger, and reload?
22:11sheppyHuh, I can&#39;t find that option...
22:12sheppyThe only options are &quot;Enable Source Maps&quot; and &quot;Enable new debugger frontend&quot;. I know I&#39;ve seen the option for pausing on exceptions in the past, but it&#39;s not there now...
22:13jwhitlockah there are two gear icons in the debugger for me
22:13jwhitlockmine is the one to the right and on the same line as &quot;Search Scripts&quot;
22:14sheppyHuh. I don&#39;t have &quot;Search Scripts&quot; either. what the....
22:14sheppyWait, there&#39;s a button in the right panel for pause on exceptions.
22:14jwhitlockAre you on the debugger tab?
22:14sheppyIt&#39;s already eneabled.
22:15sheppyOh, no. Oddly, highlighted in blue means disabled. Weird.
22:15sheppyNow it&#39;s on.
22:15sheppyAnd nothing.
22:15sheppyError still happens, but no pause
22:16* jwhitlock searches for &quot;loading failed for the script&quot; firefox
22:17jwhitlockno help there
22:18jwhitlockdoes the production one work?
22:18* sheppy checks
22:19jwhitlockit&#39;s been combined into main.<hash>.css on prod
22:19jwhitlockat least I&#39;m 90% sure
22:20jwhitlockdo you have an ad blocker that would get concerned w/ a file called analytics.js ?
22:20sheppyjwhitlock: Ohh, good question.
22:21sheppyYep. uBlock Origin blocks loads of pages called analytics.js.
22:21sheppyAnd wiki-helpful-survey.js fails on line 13 because of it, and that&#39;s killing things, looksl ike.
22:21jwhitlockok please whitelist localhost:8000
22:21jwhitlockor whatever
22:21shobsonHm. It shouldn&#39;t be failing without analytics.
22:21sheppyI will... but shouldn&#39;t the code function if analytics aren&#39;t available?
22:22shobsonWe build in a fallback.
22:22sheppyYeah, I see that... huh.
22:22jwhitlockshobson: if analytics.js doesn&#39;t run, is a TypeError
22:22sheppycheckGA is aborting.
22:22shobsonYeah but it&#39;s not suposed to be.
22:22jwhitlocksorry ignore my attempt at speaking JS
22:23jwhitlocksource of truth -
22:23sheppyYeah, I see that it&#39;s checking... but...
22:23shobsonThe global object is supposed to be instanciated regardless of the availability of ....
22:23shobsonOf the actual Google scripts.
22:23shobsonIf you block our analytics.js file you&#39;re also blocking our fallbacks.
22:24jwhitlockso line 7 of the un-combined main.js is &quot;;&quot;
22:24sheppyThat&#39;d do it.
22:24jwhitlockand yeah &quot;; is no such thing if analytics.js doesn&#39;t run
22:25shobsonThis is only a problem for people running adblockers against our development assets, right?
22:25shobsonDo we do source mapping in production?
22:25* shobson goes to answer her own question
22:26jwhitlockI think it is only a dev environment problem
22:26shobsonInclined not to fix this.
22:26sheppyOK, that&#39;s good then.
22:27shobsonBut we should say something in the docs about disabling ad blockers.
22:27sheppyIf we don&#39;t fix it it&#39;s probably worth writing down somewhere for the next person to be.
22:27sheppyyes. :)
22:27jwhitlockor change the name of the file to &quot;NOTanalytics.js&quot;
22:28sheppyWell, I still get the ReferenceError: ga is not defined. But no warning about analytics.js.
22:28sheppyAnd things are working.
22:28jwhitlockdepending on your adblock settings, you may be blocking the google analytics as well
22:30jwhitlocksheppy: are we done for now?
22:32sheppyjwhitlock: One last question: my master is behind by 150 commits. git status suggests pushing them. But shouldn&#39;t I pull them from mozilla/master instead of doing a PR and all on my repo?
22:33jwhitlockok, your local working copy is on master?
22:33sheppyYeah, I&#39;m on master now
22:34jwhitlockthis is easy to screw up, because your names for remotes may not be my names for remotes
22:34jwhitlockthe command &quot;git remote -v&quot; will show you the names and the github paths that the correspond to
22:35sheppywas already on it :)
22:35jwhitlockcan you paste the status message that says you are 150 commits behind?
22:36sheppy143 but anyway
22:38jwhitlockfirst &quot;git pull --ff-only origin master&quot;
22:38jwhitlockhmm actually
22:38jwhitlockfirst &quot;git fetch --all&quot; to make sure you are up-to-date
22:39jwhitlocksecond &quot;git pull --ff-only origin master&quot;
22:39jwhitlockthird &quot;git push&quot;
22:40sheppyThat did it. Thank you! I free you now, genie!
22:40jwhitlockum, one more thing
22:40sheppyUh, okay.
22:40jwhitlockit&#39;s probably more important that your &quot;master&quot; branch tracks mozilla/master instead of a2sheppy/master
22:41sheppyThat sounds good to me... I didn&#39;t know you could even do that. :)
22:41sheppyI mean, I thought master was special somehow. :)
22:41* jwhitlock searches for changing tracking
22:42jwhitlocklooks like &quot;git branch master -u origin/master&quot; would work
22:43jwhitlockI&#39;m assuming your are on git 1.8 or later
22:43sheppyIt did something... :)
22:43jwhitlockwhen you do &quot;git status&quot; it should say
22:43sheppyYeah, git status now says I&#39;m up to date with origin/master.
22:43jwhitlockok we&#39;re golden
22:44sheppyAwesome. Thanks for all the help. I know you have more interesting things to do. :)
22:44sheppyAlthough the info about the analytics thing was maybe vaguely helpful.
22:44jwhitlockwhen I have a merge conflict w/ the master branch, I prefer a rebase to a merge
22:44jwhitlockbut that&#39;s easier to learn about next time you have a merge conflict
22:44sheppyOK, I&#39;m letting you go now, with great appreciation for your time.
22:45jwhitlocksorry I wasn&#39;t available yesterday
22:45sheppyNot your job to be around whenever I need handholding. :)
22:46jwhitlockok, back to secret wizard stuff
22 Apr 2017
No messages
Last message: 6 days and 17 hours ago