mozilla :: #fx-team

7 Aug 2017
07:45flodrexboy: ping
07:45rexboyflod: pong
07:46flodrexboy: hi, any advice on how I can test the singlesearch panel in onboarding?
07:46flodthis doesn't seem to work https://bugzilla.mozilla.org/show_bug.cgi?id=1371531#c9
07:46firebotBug 1371531 FIXED, rexboy@mozilla.com Should add the Single search tour in the onBoarding overlay
07:48rexboyflod: For now you may need to modify some preferences.. Just a moment
07:48flodprefs for Library still work (https://bugzilla.mozilla.org/show_bug.cgi?id=1371540#c9), the one for Single search don't for me
07:48firebotBug 1371540 FIXED, gasolin@mozilla.com Should add the Library tour in the onBoarding overlay
07:50flodrexboy: never mind, this time it worked o_O
07:50flodmaybe there's some lag/cache between changing prefs and displaying that version of the onboarding
07:50rexboyflod: @_@
07:51rexboyflod: You should just need to set browser.onboarding.newtour to include singlesearch
07:51flodsorry for the unneeded ping, and thanks for putting instructions in the bug, they really help :-)
07:52rexboyflod: That's ok, you're welcome!
07:52floduhm, looks like the panel can break quite easily
07:52flodhttps://irccloud.mozilla.com/file/3MijgeJX/Schermata%202017-08-07%20alle%2009.52.13.png
07:52flodI've added some text out of curiosity, because the original one is sitting right at the bottom
07:53flodhttps://irccloud.mozilla.com/file/3SRcvB84/Schermata%202017-08-07%20alle%2009.53.03.png
07:53flodI'll file a bug
07:53rexboyWe may try to add a scroll bar there
08:28daleharveyHey, anyone have a method for debugging the awesome bar? ie so it doesnt hide when I try to use the browser toolbox
09:39Gijsdaleharvey: toggle the popup autohide stuff from the browser toolbox?
09:39GijsI think you might need something else specifically for the urlbar one but I don't remember what it is
09:42daleharveyGijs, yeh I found out about that popup autohide button yesterday but doesnt work for awesomebar
09:47* mikedeboer back!
09:56Gijsmikedeboer: welcome back! :)
09:59johannhuhm, is anyone aware of a bug where certain panels don't open in Nightly?
09:59johannhlike I'm trying to press the hambuger menu button and it doesn't open
09:59johannhbut the overflow panel opens
09:59johannhand the identity panel also doesn't open
09:59johannhand the autocomplete popup looks weird, it's transparent, only with a border
10:00johannhthis is on a fresh install of Nightly
10:00johannh(but is used to open, I just dragged the window around a bit)
10:00johannh*but it used to open
10:01johannhno errors in the browser console
10:02mikedeboerty :)
10:17daleharveyweirdly if I manage to inspect any of the awesome bar (if I press ctrl when I click the pointer it doesnt hide immediately), the awesome bar will then hide and never show again until I restart firefox
10:51Caspy7I asked this earlier, but had to leave soon after do to needing sleep...
10:52Caspy7Can anyone say if Nightly recently disabled e10s shims? Or why I'm seeing reports of many addons suddenly broken?
10:52Caspy7not all legacy addons are disable, so it's not that, yet
11:06Standard8Caspy7: I vaguely remember seeing some discussion about the sdk and something being removed that might break it. I cant remember what it was or where it was though
11:06Standard8so I could also be totally wrong
11:07Caspy7\_()_/
12:03johannhnhnt11: remember when you wanted to add a BrowserTestUtils function that loads a URI and waits for load, did you ever end up doing that or filing a bug?
12:04nhnt11let me check
12:05nhnt11johannh: I don't think I did :(
12:06johannhnhnt11: np, I was just curious
12:06johannhI'm busy too :)
13:45GijsTIL: we now have an add-on hexapod overlord
13:47johannhhaha that was the best bug title
13:54Gijsmikedeboer: so is SessionStore.getCurrentState() not going to use lazy state?
14:03mikedeboerGijs: lazy state?
14:03Gijsmikedeboer: well, does calling getCurrentState() enumerate all windows and then tabs and then return stuff?
14:03Gijsmikedeboer: or does SessionStore cache something somewhere?
14:05mikedeboerGijs: it'll iterate the windows anbd get the cached data from it if it's there. Otherwise it'll collect it the first time it's called the `DirtyWindows` tracking
14:05mikedeboerGijs: and a window is marked dirty as such when the 'TabOpen' listener is invoked
14:19zombiei for one welcome our new Cthulhu overlord
14:32Standard8kitcambridge: markh: whats the tps extension in sync? Is that production code or test?
14:33kitcambridgeStandard8: tps is all test code
14:34Standard8kitcambridge: bugs in the sync component?
14:34kitcambridgeyup
14:34Standard8ok thanks
14:34Standard8looks like its using a lot of sync bookmark apis
14:35kitcambridgewe need to asyncify them? :-)
14:35Standard8kitcambridge: well, only if you want your tests to keep running ;-)
14:35Standard8who needs tests anyway?
14:36* Standard8 hides
14:36kitcambridgehehehehe
14:36kitcambridge#yolo
14:37Standard8oh typical, theres already a bug there :-)
15:05Gijsmikedeboer: I'm literally pushing a patch to that history bug right now
15:05mikedeboerGijs: whoops
15:05Gijsno worries, you couldn't know
15:05Gijsr?you
15:05mikedeboerI just noticed the typo abnd thought: why not push a patch?
15:05Gijs;)
15:06GijsI've been too busy
15:06Gijsbut also, the library-related typo is more recent
15:06mikedeboerGijs: ok... but that made the history subview also not work for me
15:06mikedeboer(obviously)
15:07mikedeboeranyway, yougot that covered in your patch as well, so no worries
15:07Gijs:D
15:07GijsI took a bit longer because I wanted a test
15:07Gijs(bad idea, I know)
15:08Gijsmikedeboer: speaking of which, please lmk if I need a waitForCondition or something to wait for that history entry to appear in the test
15:08GijsI assume that the placesview is guaranteed to be rendered by the time viewshown fires
15:08Gijsbut if not then I guess I can add the waitforcondition
15:08mikedeboerGijs: btw, if you change `var PlacesPanelview = class extends PlacesViewBase {` to `this.PlacesPanelview = class extends PlacesViewBase {`, it'll work and the lazy getter will tooo
15:09Gijsmikedeboer: I can amend the patch to do that, sure
15:09mikedeboerGijs: yes, it's an async API
15:09mikedeboer*sync API, I mean. The opposite
15:09Gijs"yes" as in I do need a waitForCondition?
15:09Gijsoh
15:09Gijsok
15:09GijsI was confused already :)
15:17Gijsmikedeboer: for the suggested change to the class definition, that makes eslint grumpy
15:19mikedeboerGijs: also with `var PlacesPanelview = this.PlacesPanelview = ...`?
15:20Gijsyes, it looks like it thinks the variables are no longer exported from the browserPlacesViews.js file
15:21Gijsit doesn't just complain about PlacesPanelview, but also about the other things in that file
15:21Gijsfor some reason
15:21Standard8that sounds strange
15:23Standard8oh huh
15:23Standard8we only match against this\.__defineGetter__
15:23Standard8ah bug 1242893
15:23firebothttps://bugzil.la/1242893 NEW, nobody@mozilla.org Change the import-globals-from eslint custom rule to also register properties added to the top-level
15:24Standard8although browserPlacesViews.js isnt a jsm
15:24mikedeboerGijs: ^ but I also think it's weird that it complains about the other exports there... are you linting the entire dir?
15:25Standard8please file bugs with examples if you think theres real problems with eslint here
15:25Standard8we need to get them fixed
15:25mikedeboerGijs: because for me it does not error...
15:30Gijsmikedeboer: I'm using ./mach lint --outgoing
15:30Gijsmikedeboer: re your question about the rootElt thing, did you read https://bugzilla.mozilla.org/show_bug.cgi?id=1385083#c10 ? :)
15:30firebotBug 1385083 ASSIGNED, gijskruitbosch+bugs@gmail.com History button (and history panel in library, after bug 1354117) doesn't show the Recent History
15:30GijsI think that's basically the explanation you want
15:33mikedeboerGijs: ok yeah, that's sufficient... weird that I wrote it this way using .viewElt...
15:33mikedeboeroh well
15:33mikedeboerGijs: that must be a bug in --outgoing then, because `./mach eslint browser/components/places/` yields 0 errors
15:36Gijsmikedeboer: after those changes you suggested?
15:36Gijsoh
15:36Gijsno, the errors are in browser-places.js
15:36Gijs./mach eslint browser/base/content/browser-places.js
15:36Standard8Gijs: mikedeboer: outgoing will check only the outgoing committed changes iirc
15:37Standard8workdir for the work directory
15:37Gijsand that dir command also breaks
15:54mikedeboerGijs: but... browser-places.js? odd... and I've got errors all over the place too when I invoke `./mach eslint browser/`
15:54mikedeboerStandard8: ^ Is that expected?
15:54Standard8shouldnt be
15:55Standard8mikedeboer: pastebin?
15:56Gijsmikedeboer: yeah, it seems using any of the variables 'exported' from browserPlacesViews.js triggers a no-undef warning in browser-places.js
15:56Gijsditto for PanelUI.js actually
15:56Gijshm
15:56Standard8now that might be expected
15:56Gijsmaybe the 'window' plugin thing isn't working right?
15:57Standard8hmm, maybe not
15:57Gijsoh, same errors if I revert the changes
15:57Gijswell then.
15:58Standard8so on master, I have no issues
15:58GijsStandard8: how do I find out if this will turn the tree orange?
15:58Standard8what do I need to apply to see this?
15:58Gijswhat is "master"
15:58Standard8default...
15:58Standard8latest m-c head
15:58Gijshash?
15:59* Gijs is on bb8de16ce00c
15:59Gijson windows
15:59Gijsmozillabuild 3.0, the lot
15:59Standard847248637eafa9a38dade8dc3aa6c4736177c8d8d
16:00Standard8but bb8de16ce00c should be fine as well
16:00Standard8according to automation
16:01Gijsdoesn't make any difference
16:01Gijseslint 3.19.0
16:02Standard8Gijs: whats the output?
16:02Gijsaccording to ./mach eslint --version
16:02Gijshttps://pastebin.mozilla.org/9029064
16:02GijsStandard8: ^^
16:02Gijson m-c itself I just get more errors, rather than fewer, for that file
16:02Gijsbecause PlacesPanelview is accessed twice instead of once
16:03Standard8Gijs: so do you see issues in other files as well, or is it just that file?
16:03Gijsyes, other files as well
16:03GijsPanelUI, gSafeBrowsing, gSync, everything is "not defined"
16:04Gijs302 errors in browser/base alone
16:04Gijs*301
16:04Gijsnot that it matters...
16:04Standard8hrm
16:04Gijsnode 6.10
16:04Gijsnpm 3.10.10
16:04Gijshow do I check if the eslint plugins are up-to-date and so on?
16:05Standard8in theory it should do it itself
16:05Standard8your npm / node versions are out of date wrt mozillabuild as 3.0 comes with node 8. something
16:05Standard8but I dont think that should matter
16:06RyanVM6.10 is the LTS version IIRC
16:06Standard8Gijs: look in node_modules/
16:06RyanVMGijs: |npm update| ?
16:06GijsI'm using mozillabuild 3.0
16:06Gijsso how can they be out of date?
16:06GijsStandard8: going to need more than "look in node_modules"
16:06RyanVMyou should be on node 8.1.4 and npm 5.30 then
16:06Standard8Gijs: in theory the eslint-plugin-mozilla ones are symlinked
16:06Gijsthere's a shitload of stuff in there
16:06GijsStandard8: windows doesn't have symlinks.
16:07Standard8well, node handles local links
16:07Standard8or should do...
16:07RyanVMGijs: yeah, we had to flatten the node_modules directory because the amount of directory recursion was breaking installer packaging otherwise (yay Windows)
16:07Standard8Gijs: windows does have symlinks: https://msdn.microsoft.com/en-us/library/windows/desktop/aa365680(v=vs.85).aspx
16:08Gijsthey don't show up when using ls -al
16:08Gijsanyway
16:09Gijsthe different node and npm versions are because my path variables had my own node installs first
16:09Gijsbecause that was necessary prior to mb 3.0
16:09Gijsbut removing them from .profile and opening a new shell and, having verified more up-to-date versions, ./mach eslint --outgoing is still complaining
16:09Standard8can you zip your node_modules directory up
16:09Standard8then ./mach eslint setup
16:09Standard8?
16:10Gijshttps://pastebin.mozilla.org/9029065
16:10Gijsthat wasn't what you asked, but that's npm update's output
16:10Gijswhich is, uh, unhelpful
16:10* Gijs mutters about variable naming
16:10Gijs"cb() never called"
16:10Gijsthanks for that...
16:10Standard8huh, i shouldnt be calling that file
16:11GijsStandard8: ?
16:12Standard8Im surprised npm update is calling into createExports.js
16:12GijsStandard8: ./mach eslint --setup didn't fix anything
16:13Standard8hrm
16:13Standard8Gijs: ok, next option, nuke node_modules, then setup again
16:14GijsStandard8: ok, you're getting node_modules via email...
16:14Standard8sure
16:15Gijsannnnd it's fixed
16:15Gijswell, that was fun!
16:15Standard8ah interesting
16:16Standard8mikedeboer: ^^^ so rm node_modules & re-run setup
16:16mikedeboerStandard8: ah ok :P
16:17Standard8wish I knew why
16:18Standard8Ive got a feeling some of it is to do with npms package management
16:21Gijsnpm update stopped complaining now, too
16:21GijsStandard8: looks like you didn't get my email because gmail decided it has a virus
16:21Standard8heh
16:21Standard8Gijs: TestPilots send file?
16:22Gijsno
16:22Gijsit looks like google doesn't allow you to send .js files zipped
16:22Gijs(or unzipped)
16:54mikedeboerStandard8: perhaps having an ESLINT_CLOBBER file might be an idea?
16:58Standard8mikedeboer: yeah, I was pondering if some sort of clobber mechanism might be a good idea. atm though I dont know what will trigger it
16:58Standard8mikedeboer: or rather, what would trigger the need
16:58Standard8I think a lot of it is just updating from old stuff
16:59mikedeboermyeah, true... I don't really know either - I was hoping you'd have a heuristic in mind from experience
16:59mikedeboer:P
19:33Gijsgah, heisenbugs :(
21:08Gijsjaws: so my trypush is more orange than yours. Most of the test fixes look trivial. When I make them tomorrow, do you want to re-review?
21:08Gijshttps://treeherder.mozilla.org/#/jobs?repo=try&revision=272e40c27527&selectedJob=121483312
21:08jawsGijs: looking...
21:09jawsGijs: this view should be more helpful for you by the way, https://treeherder-manifest.herokuapp.com/?repo=try&revision=272e40c27527ef9573a1000fc79ffffe0407414b
21:09Gijsomg
21:09Gijswho made that?
21:09Gijswhen did that become a thing, and how did I not hear about it until now
21:09Gijs?
21:09RyanVMit's the experimental test-centric view
21:09RyanVMdrop-down on the RHS :)
21:10jawsGijs: you can get to it from the ^
21:10jawsyes
21:10jawsi found out about it in SF
21:10jawsfor the all-hands
21:11* Gijs chalks another one up for missing that :(
21:14Gijsjaws: anyway, that's pointing out that browser/base/content/test/popupNotifications/browser_popupNotification_no_anchors.js is failing a lot, and that intermittent is 4 months old with almost no hits
21:16* Gijs wonders how he's breaking that :s
21:19jawsGijs: i only found out about it while sitting at a table with someone. there wasn't a presentation
21:20Gijsjaws: sounds like you'll get another review anyway, sorry!
21:20Gijsbut tomorrow - first I need sleep :)
21:20jawsGijs: no problem
21:21jawssee you tomorrow, have a good night
21:35MossopIs the library button photon-structure?
21:49adwMossop: yes
21:56jawsadw: how do you recommend i change the PageAction API to allow a page action to have custom markup for the urlbar action?
21:56jawsadw: i need to do something like #star-button-box for the pocket button
22:00adwjaws: yeah, probably how the star button is implemented: it's entirely in markup (in the urlbar, that is), and you set _urlbarNodeInMarkup to true when you add the action to gBuiltInActions in PageActions.jsm
22:00adwand urlbarIDOverride too
22:00jawsadw: okay, the only odd thing is that pocket is not one of the built-ins
22:00adw........ oh yeah
22:01adwhmm
22:01jawsadw: i suppose we could just do it like the star button anyways
22:01jawsand just put it in the markup
22:01adwyeah
22:01jawsotherwise i'm looking at changing how _makeUrlbarButtonNode works
22:01adwor you could build it via js in the pocket code and add it
22:01adwor use an overlay
22:01adwand that might work?
22:02adwbasically _urlbarNodeInMarkup just says, "take a hands-off approach to the urlbar button for this action"
22:02adwso you could still make your dom however is convenient
22:02adwwhether that's via js, a xul overlay, or right in browser.xul
22:03adwi'd rather do that than complicate _makeUrlbarButtonNode tbh
22:03jawsadw: ok, also, we have an onShowingInPanel() function, but nothing for when we stop showing it. am i missing something or should i add it?
22:03adwyou mean when the panel is hidden?
22:03adwor when the action is removed?
22:04jawsadw: i need to be able to set an attribute when the pocket panel is open and then remove that attribute when the panel closes
22:04jawsto change the pocket icon
22:04jawswe only need this if the icon is in the urlbar. we don't need to do anything if it is in the panel
22:04adwok. let me see
22:06adwjaws: Subview has an onShowing, which will be called when the pocket panel is showing
22:06adwonShowingInPanel is different and not what you want, sounds like
22:06adw(that's called when the main page action panel is shown)
22:06adwonShowing is passed the <panelview> node
22:06adwso from that you could get the panel and then add popuphidden, etc.
22:07jawsadw: okay cool i&#39;ll do that
22:07jawsthanks
22:07adwor you could add a new onHiding or something to go along with Subview&#39;s onShowing
22:07adwi didn&#39;t need that which is why it&#39;s not there
22:08jawsyeah, i was thinking of that
22:09adwjaws: sorry for breaking the animation to begin with though. didn&#39;t even realize it
22:09jawsif another consumer needs it then it would make sense
22:09jawsyeah, i guess i should have added tests for it
22:09jawsthough the tests could be complicated if they actually tried to make sure the animation actually ran
22:09jawsand its likely they would get ignored due to the change in implementation
22:13adwjaws: when i said ^ &quot;i&#39;d rather do that than complicate _makeUrlbarButtonNode tbh&quot; -- actually if animations for these icons is going to be the rule and not the exception, then maybe that should be built in. i don&#39;t mean we should do that now necessarily, but something to think about in that case
22:13jawsadw: well, we will disable the animation if the user changes the toolkit.cosmeticAnimations.enabled pref
22:13jawsso doing it in JS isn&#39;t so bad
22:14jawsi do appreciate you keeping the star button animation working, i see you had to do a bit of work to keep that working
22:16adwif you want to take a shot at building in animation-related dom setup in js and then use that for the star button too, that&#39;s cool with me
22:17adwespecially if you think that&#39;d be easiest for you
22:17jawsyeah i might do that in a separate bug
22:17jawsthanks for the tips, this is easier now
22:17adwok
22:17jawsi think i was looking at it the wrong way before
22:17jawsi&#39;m using onPlacedInUrlbar() now to start building the DOM in JS
22:18jawsthat sound right to you?
22:18adwhmm
22:19adwi think that might not be the best thing, because when that callback is called, the node will have already been created for you
22:19adwwhat i was thinking was
22:20adwyou would just create your dom unconditionally, immediately
22:20adwand then add your action with _urlbarNodeInMarkup set to true
22:20jawsadw: how will i make sure that my dom is in the correct place regarding order of page actions?
22:21adwPageActions will move it to the right place, basically
22:22jawsokay
22:22jawsso do this in PocketPageAction.init() ?
22:22jawsbefore calling PageAction.addAction?
22:22jawsPageActions.addAction*
22:23adwjaws: yeah (sorry, had to look up the code)
22:23jawsno worries, thanks for the help
22:23jawsokay i&#39;ll do it that way. thanks
22:25adwhmm, but you don&#39;t have a browser window at that point
22:25adwyou could do it in onPlacedInPanel
22:26adwthat will be called for each window that the action is added to...
22:26adwi think that should work, since actions are placed in the panel before they&#39;re placed in the urlbar...
22:26adwjaws: ^
22:27jawsthanks for the ping
22:27jawsyeah i was just looking around now because of that
22:27jawsadw: or could create a new onBeforePlacedInUrlbar
22:27adwtrue
22:28adwor maybe a more general onBeforePlacedInWindow?
22:28adwcalled before both the panel and urlbar callbacks
22:29jawsokay i can do that
22:29adwok
22:30adw(we&#39;re going to end up reinventing CUI, which is what i should have used/extended in the first place :-/ )
22:31jawsyeah, i was curious why we didn&#39;t re-use it
22:32jawsand just treat this as another area
22:32adwyeah. i&#39;m dumb is the reason
22:32jawsthough the fact that the buttons can show up in two places might confuse CUI
22:32jawslol no if you&#39;re dumb i&#39;m dumber
8 Aug 2017
No messages
   
Last message: 9 days and 23 hours ago