mozilla :: #fx-team

14 Mar 2017
09:16johannhyou know what would be really cool? try machines live-streaming and recording their screens
09:18freddybwell, you can create screenshots and dump them into the folder of things that survive the machine's dead
09:18freddybdeath*
09:19johannhyeah but I want the recording unfortunately
09:19johannhI could create a lot of screenshots, say 30 per second
09:19johannhbut I have the slight feeling that that would alter the test conditions
09:20freddyboh, you need that much? hm. yeah. :/
09:20johannhit's not important, just had this wonderful day dream
09:20freddyb:)
09:20johannhI can still use a loaner with a VNC connection so it's almost as good
09:21johannhexcept for the cases where you can't reproduce on the loaner anymore
12:35Gijsmak: thanks for the super quick turnaround on the insertTree review.
12:35Gijsmak: I'm a bit confused about the index stuff... do I need to do the subquery stuff in my patch? AIUI from both the code and, I think, your comment, the existing code (insert() and others) have the same problem, right?
12:56makthey are only inserting one entry
12:56makbut it's possible. just it's more likely to happen here because this insertion can take a longer time
12:58makbut good point, it may be worth filing a bug
12:59mak(it will be harder to handle when the insertion happens in the middle)
13:16Gijsmak: fair
13:16Gijsmak: so I take it you do expect me to fix this for the insertTree case in this bug?
13:17makyes, since we are only appending it should be trivial enough to fix that it's worth it
13:17makif we'd be inserting at a custom index instead, it would be hard enough to be worth a follow-up.
13:25mconleyMattN: hey - thanks for sending out the meeting email! I totally forgot. :/
13:35Gijsmak: ok. The other thing I was wondering is this moz_assert stuff for failure of generating a guid
13:36makyeah?
13:36Gijsmak: is there a sane way to moz_assert on an nsresult?
13:37Gijsmak: or is the best I can do MOZ_ASSERT(NS_SUCCEEDED(rv), "Shouldn't fail") or something
13:37Gijsmak: or did you have something else in mind?
13:37makhm, I remember MOZ_ASSERT_SUCCEEDED, but I think that's to be used in an if
13:38makfwiw you could just if (NS_FAILED(...)) { MOZ_ASSERT(false, "blabla"); setIsVoid; return } not that important
13:39makmaybe I'm misremembering. let me check one thing
13:39makah ok it's MOZ_ALWAYS_SUCCEEDS
13:40makit's evaluated in release, it asserts in debug
13:40makbut, in your case you also need to setIsVoid and thus you may have to still use a plain if
13:40Gijsmak: OK. And return NS_OK in the failure + setisvoid case?
13:41makyes
13:52mikedeboerdao: ping
13:52daomikedeboer: pong
13:54mikedeboerdao: hey! wrt bug 1347113, for my understanding, why is the margin-inline-end not necessary anymore for .checbox-check? Is that because the default is good?
13:55firebothttps://bugzil.la/1347113 ASSIGNED, dao+bmo@mozilla.com Use CSS outline instead of border for XUL checkbox focus ring on Windows
13:56* Gijs fights the compiler
13:57daomikedeboer: I don't understand your question. my patch *adds* margin-inline-end for .checkbox-check (to replace the .checkbox-label-box weirdness)
13:58mikedeboerdao: *ahum* Ooo, so that's what the green color means? ;-P
13:58mikedeboerdjeez
14:59mconleyMardak: hey - I hope it's cool I added some Activity Stream stuff I noticed to the meeting notes.
14:59Mardakno problem. i'm actually trying to figure stuff out myself :p i was out on joco cruise last week ;)
15:00mconleyoh, welcome back!
15:00mconleymeeeeeeeting time
15:07Mossopflo-retina++
15:08pastsfoster: welcome back!
15:08RyanVM|mtgsquib: welcome aboard!
15:09Gijsmak: so... this subquery thing to determine the index
15:09Gijsmak: I don't have a parentId
15:09makwell, you are inserting a parentId...
15:09* RyanVM|mtg stores away that Red Eagle comment for later use
15:10Gijsmak: yes, that's using a subquery :)
15:10makwhen you fetch the root, there is probably also the id somewhere
15:10makin a _property
15:10Gijsmak: sure, but the parent might not be the root
15:11makthis index problem exists only for things that you are inserting in the "root"
15:11makfor anything else you can use your normal index pre-calculation
15:11Gijsmak: I tried doing basically: insert ... values (..., (SELECT id FROM moz_bookmarks WHERE guid = :parentGuid) AS parentId, ... SELECT ... WHERE parent = parentId
15:11Gijsbut that doesn't seem to work?
15:11Gijsmak: hm, that would work, though it feels fragile...
15:12makwhy?
15:12Gijsmak: mostly because the query is so far away from where we do the index setting
15:12makI still don't follow
15:12Gijslike, yes in principle the index is only null when we're inserting in the root
15:12Gijsbut...
15:12makthe index setting should just set NULL as index for any item in the root
15:12makthe query will just recalc indexes for anything having NULL as index
15:13Gijsmak: but at that point it assumes that NULL => you're in root
15:13makyes
15:13GijsI mean, to be fair, the validation code breaks if you set index to null
15:13Gijsso I guess that guarantees no other items will have it set :|
15:13makmakes sense :)
15:13makok, so you want the query to figure out if it's in the root, rather than using null
15:13Gijsmak: yeah, maybe
15:14Pikeand yet another new engineer right there
15:14_6a68looks like mardak is getting a tiny new contributor up to speed
15:14_6a68Pike: haha jinx
15:14makthe only way is to check parentId, and for that you need to bind the _itemId you get out when checking if the parent is valid
15:15makif you don't want to rely on an index hack (that may mean setting index to NULL after the input checking)
15:17Gijsmak: looks like we don't restrict index to not be null on the sql level
15:17makthe schema is old and was designed by blind monkeys
15:18Gijsthis is where I'm very glad I'm facemuted for this meeting
15:18Gijsblind monkeys, teehee
15:23mikedeboermconley: re spinners, I filed bug 1345435
15:23Gijsmak: next issue, sorry, this stuff is messy
15:23firebothttps://bugzil.la/1345435 NEW, nobody@mozilla.org Don't show loading indicator (spinner) when restoring an older docShell
15:23Gijsmak: the URL objects vs. url strings and Set and whatnot
15:23makyes, just use plain href, imo
15:23Gijsmak: turns out maybeInsertManyPlaces also relies on URL objects
15:23Gijsmak: specifically, it calls into PlacesUtils.getReversedHost()
15:23Gijswhich definitely needs an actual URL object
15:23Gijsmak: I'm tempted to just use an array rather than a Set
15:24Gijsmak: seems like many duplicate bookmarks with identical URLs are unlikely to be a common case anyway
15:24Gijsmak: but then use actual URL objects
15:24Gijsmak: thoughts?
15:24Gijsmak: to be fair, I initially ended up doing what you suggested and that simplified updateFrecency
15:24Gijs(I basically made it take an array of strings, which is a lot easier to turn into a SQL IN (...) query...
15:24Gijs)
15:25makblah, leave it as it was
15:25makI missed the reversedHost depndency
15:25Gijsyeah, me too :(
15:25Gijsmak: we could avoid the extra iteration if we just used an array rather than a Set, though
15:25Gijsmak: do you think the Set really gets us something?
15:26makIt's unlikely deduplication has an interesting effect here
15:26makexactly as you said it's unlikely a user will have many duped bookmarks (maybe some)
15:26Gijsmak: right, in which case we can avoid the extra iteration
15:26Gijsmak: I'll do that instead, thanks for bearing with me
15:26makso, yes, you could even just avoid deduping
15:27sfosterdao: are you thinking of something like inferFromText({ toolbarid }) or something.. what would the reason argument look like?
15:28sfosterunrelated: I keep getting these extra heads when changesets I've bookmarked get merged to central. It seems the hash changes in the autoland / merge process?
15:28sfosteris that normal. What is the expected workflow with bookmarks?
15:29Gijssfoster: yeah, autoland rebases
15:29Mossopsfoster: That is normal for now, I just prune them afterwards
15:29sfosterok
15:29Gijssfoster: you can use hg prune to get rid of the dead csets that you developed with
15:29* sfoster looks up difference between hg prune and hg strip
15:29GijsI think we want to have 'hg evolve' support for autoland / the server at some point
15:30MossopYeah, that's the plan
15:30sfosterok thanks
15:30Gijshg strip is awful is the TL;DR
15:30Gijsit creates backups in a . directory somewhere daft
15:30sfosterright
15:30MossopYeah, prune is much nicer assuming you have the evolve extension enabled, which you should
15:30sfostertho' I did have cause to use that backup last week. Don't ask :)
15:31MossopWith prune the changeset is still in your repo, just hidden so you can still get back to it
15:31Gijsfor prune with evolve you can use "hg touch --hidden --rev blah
15:31Gijsright
15:31Gijs(you can find the hidden revs by passing --hidden with things like 'hg log' and other commands)
15:33jawsmikedeboer / Gijs / dao: do you have anything that should be added to qx section?
15:33Gijsjaws: adding
15:33jawsthanks!
15:38johannhmconley: sorry, have to drop. I'll be back in a couple of hours, let me know if you need help sorting stuff the
15:38johannh*then
15:38mconleyjohannh: sounds good, thanks!
15:40Pikeflod: have you looked at the l10n-side of the prefs work yet?
15:40flodI've seen stuff from Michelle last month
15:43flodjaws: have a tracking bug for the new prefs?
15:44jawsflod: this is the meta bug, https://bugzilla.mozilla.org/show_bug.cgi?id=1324168. look at list of deps there
15:44firebotBug 1324168 NEW, nobody@mozilla.org [QX cluster] Preferences refresh 2017
15:44flodthanks
15:44flod(d'oh, I was already following it, too many bugs)
16:04mconleyandym: hey - do you also have a bug for the "virtual file system on top of indexedDB" work?
16:07mkaplyIs there some way to figure out why mozreview won't let me land commits? Try build is passed and it is r+
16:07mkaplyhttps://reviewboard.mozilla.org/r/114304/diff/10/#
16:09mconleymkaply: open issues on https://reviewboard.mozilla.org/r/114304/
16:09Mossopmkaply: Do you have level 3 commit access?
16:09mkaplyAh, I need to mark the old open issues as resolved. Got it.
16:10mkaplyI somehow assumed a new patch would do that automatically.
16:10mkaplymconley: Tx
16:11mconleymkaply: np
16:31mikedeboerdolske: our etherpad is thrashing my Fx content process :(
16:31makGijs: great job
16:32Gijsmak: worried about that index increment stuff that I left around
16:32makthe problem there is that it's not easy to test
16:32Gijsmak: like, that means nothing is hitting that subquery
16:32Gijsand yeah, tests fail if I remove that :(
16:32makwell, you are overriding null with valid indices...
16:33dolskemikedeboer: hmm, I'll archive the notes after today's meeting then
16:33makbtw, it's the only problematic thing left
16:33dolskemikedeboer: also, meeting ping :)
16:33Gijsmak: I know, but need to see why tests are failing now - going into another meeting, but I hope to have a fix in the next hour or so
16:34maksure, if you need another review round no problem on my side, I'm satisfied with the API in the end :)
16:38pdolGijs: Do you know what to look for in telemetry to determine opt-out rates (and whatever else we store) for automigration?
16:39Gijspdol: yes
16:40Gijspdol: e.g. https://mzl.la/2mG1Yq5
16:43pdolGijs: thanks. Can I determine something like user Undos as a percentage of those who have the Undo offered?
16:44pdolEg. 30% of users chose to Undo by clicking the No thanks button
16:44pdolThat is, 30% of users who are automigrated and offered the Undo option.....
16:44Gijspdol: yeah, so, that graph is kind of what you want
16:45Gijspdol: the telemetry dist.html UI is kinda rubbish because it crops the label
16:45Gijspdol: here's the full description of that histogram
16:45Gijs "description": "Why the undo functionality of an automatic migration was disabled: 0 means we used undo, 1 means the user signed in to sync, 2 means the user created/modified a password, 3 means the user created/modified a bookmark (item or folder), 4 means we showed an undo option repeatedly and the user did not use it, 5 means we showed an undo option and the user actively elected to...
16:45Gijs...keep the data. The whole thing is keyed to the identifiers of different browsers (so 'chrome', 'ie', 'edge', 'safari', etc.)."
16:45pdolGjis: yeah, I see that. Awesome, thanks.
16:46Gijspdol: the only caveat here is that it's in principle possible for users to get automigrated and never see the prompt (e.g. if they crash on startup and don't use about:newtab/about:home), which is very much an edgecase but in principle possible
16:46pdolGijs: That's fine. Just looking for rough numbers.
16:46Gijspdol: they're not very good, on beta 53 (which is what that graph shows)
16:47Gijspdol: ie looks like ~45% of folks choose to undo
16:48pdolGijs: and 0 means we used undo. What's that based on? A timeout?
16:49Gijspdol: no, that's users clicking the undo button
16:49Gijspdol: the timeout case is 4, and we don't undo in that case
16:49Gijs5 is actively saying "keep this"
16:49pdolGijs: oh, sorry, missed the line break for option 5. Makes sense.
16:49Gijspdol: so, interesting thing here...
16:50Gijspdol: obviously, option 4 being based on showing this prompt on 3 separate days + telemetry latency introduce latency
16:50Gijspdol: so the graph for 52 looks quite different
16:50Gijspdol: https://mzl.la/2mG3Dfj
16:51pdolGijs: not too surprised.
16:52Gijspdol: well, right, but that graph shows that more people have the notification bar "expire" (ie we stop asking after 3 days, and keep the data) than people actively picking "get rid of this imported data"
16:52pdolI do think that 1/3 of users keeping it is a decent number (as long as it isn't adversely affecting the broader swath of users.
16:52Gijspdol: so the total is more like 60-70% of people keeping - it's the sum of 4 and 5
16:52Gijsfor 52, anyway
16:53Gijspdol: IIRC we only had the new wording of the notification bar for 53, though
16:53pdolRight, exactly
16:53Gijswell, the performance right now looks worse on 53 than on 52 :P
16:53Gijsin terms of people choosing to undo
16:53Gijsdon't know how much that'll correct over time due to the latency
16:54pdolGijs: did we get probes in for hangs?
16:54GijsI think so
16:54Gijspdol: yeah, https://mzl.la/2mG4aOs
16:56mconleyApparently my Firefox Headlines emails are getting too beefy. I'm in the firefox-dev moderator queue for exceeding the size limit :/
16:59MattNmconley: One GIF is enough Mike. How many did you attach? :P
17:00mconleySo many
17:00mconleyso so many
17:00sfostershorlander: when is a good time to talk osx focus rings? I'm still seeing some glowy blue highlights vs. the flatter style. I can probably audit, but maybe you are aware of bugs covering this stuff already?
17:01sfostergood to see you again pdol :)
17:06Gijsmak: so... bad news... we need some way to update the items after insertion so they contain the right indices.
17:07Gijsmak: err, specifically, the JS objects that we're returning from insertTree()
17:07Gijsmak: right now they'll just have `null`
17:07Gijsmak: which the tests don't like
17:07pdolsfoster: ditto!
17:07Gijsmak: but I don't really see how to get that information without doing a separate fetch, which of course I would prefer not to do because of perf...
17:11makGijs: let's just return the "guessed" indices for now, and file a follow-up to figure out a strategy...
17:11Gijsmak: OK.
17:12makit's not critical, it's something that can happen in edge cases, and it's unlikely the consumer can cause damage with a broken index
17:12makso we can take a bit more time for that
17:12squibRyanVM|mtg: thanks!
17:13shorlander@sfoster: where are you still seeing the blue glowy ones?
17:13shorlanderI am not aware of additional bugs off-hand, but they might exist!
17:14makGijs: also notifications, should not notify a null...
17:15Gijsmak: right :)
17:16sfostershorlander: I just saw one on an alert box. And on notification close buttons I think. I'm not in front of my mac right now.
17:17sfosterBut I can also search for -moz-focusring and spot some box-shadow values that arent using the variables/macros and which implement glowiness.
17:17Gijssfoster: my impression is that we should have updated all of them but didn't, when yosemite was released
17:17Gijssfoster: and by we I might mostly mean 'me' :)
17:18Gijssfoster: shorlander can confirm/deny, of course, but I'd be surprised if there was any kind of intentionality about this
17:18GijsI certainly don't recall any
17:18sfosterGijs, shorlander I'll come up with a list. Maybe I'll just come up with a patch and get ui review
17:18Gijssfoster: patch sounds good to me! :)
17:19sfosterI guess the main thing I need to know is if we have different shadow weights for different functions/areas
17:19sfosteror if I should just apply the same focussed box shadow everywhere
17:22Gijssfoster: I'd start with the same one everywhere and then ui-review can always do something else... the shadow certainly looks the same everywhere on native OS X to me, but I could be missing something :)
17:23shorlanderThe original intent was to mimic the native focus rings: https://cl.ly/2j173S1g0v0T
17:23shorlanderWhich is a 4px border with 2px inside the element and 2px outside
17:24sfosterGijs: shorlander ok I'll run with that and see how it pans out.
17:27Gijsshorlander: IIRC some of this stuff is dpi-specific :(
17:28Gijsshorlander: so on my hidpi mac I see a much larger border outside the search field in Calendar.app than in your screenshot
17:32shorlanderGijs: ah, interesting. Most things are just doubled. Looks like they are doing something specific there.
17:34shorlanderIt looks like the outer part is 3px (normal DPI) and gets doubled to 6px (retina DPI), but the inner part is 1px regardless of DPI.
17:38Gijsshorlander: and by 'px' you mean screen pixel :)
17:38Gijs(not CSS px)
17:39Gijskitcambridge: ping?
17:39Gijskitcambridge: would it be possible for you to have a quick look at the sync delta stuff in the patch in https://bugzilla.mozilla.org/show_bug.cgi?id=1344282 to make sure I'm not crazy?
17:39firebotBug 1344282 ASSIGNED, gijskruitbosch+bugs@gmail.com Add an insertTree API to Bookmarks.jsm to allow batch-inserting a set of bookmarks
17:41kitcambridgeGijs: hi! sure, i'll take a look now
17:41Gijskitcambridge: https://reviewboard.mozilla.org/r/118240/#comment158060 - basically, I'm inserting multiple items with a single source, and reusing the syncStatus and syncChangeDelta determined based on that source
17:46kitcambridgeGijs: that looks excellent. your loop will take care of the items, `setAncestorsLastModified` should take care of bumping the parent
17:47Gijskitcambridge: fab, thanks!
17:48kitcambridgenp! it'll be so nice to have this api, too :-)
17:49kitcambridgeinstead of inserting one...by one...by one...by one...
18:02Caspy7dolske: if you find that you want to kick/bad Operator in #firefox I'll have no issue with that
19:32pdolPreseed Awesome meeting notes: https://docs.google.com/document/d/1xGIB58R_NYkeIRnjhpUNB0ArcECDvQk_w4GcHnJL7TI/edit
20:20Gijspdol: so do you want me to file a tracker bug for the funnelcake and whatever engineering work comes out of that, or are you taking point on that? :)
21:49manotejmekamattn: jaws (Jared) informed me that you have written the code for subdialogs.js file with in preferences. I have few questions.
21:49MattNmanotejmeka: right, okay
21:50MattNit got more complex after I wrote it btw. but I will try answer
21:52manotejmekamattn: It looks the when a dialogue pops up the main conent of the dialogue box is with an element called #documents which is XULDocument object. I need to treverse the tree from class="groupbox-body" element and be able to access the textNodes, label nodes and all child nodes. I have having trouble walking the nodes to get to the id="PermissionsDialogue"
21:52manotejmekaelement. I want to know how I can walk the tree to get to the all child nodes of class="groupbox-body" element. https://usercontent.irccloud-cdn.com/file/0bnBLLeV/Screen%20Shot%202017-03-14%20at%204.41.35%20PM.png
21:53manotejmekamattn: I am having trouble *
21:54* MattN guesses there is anonymous content involved
21:54MattNone sec
21:55MossopWell it's an inner frame
21:55MattNwell he said child of class="groupbox-body"
21:55MattNor she
21:55jawsmanotejmeka: can you use contentWindow?
21:55MattN*sorry
21:56manotejmekamattn: It is a he :)
21:56MattNok, thanks
21:56MattNdidn't want to assume
21:56MattN(like I did for a second)
21:56MossopThe id=&quot;PermissionsDialog&quot; is in a separate document to the class=&quot;groupbox-body&quot;. When traversing you have to get to the <browser> element then go to browser.contentDocument and traverse from there
21:57manotejmekajaws: What is the contentWindow?
21:57MattNright, if you want descendants of that child then do what Mossop and jaws are saying
21:57manotejmekalet me check one sec
21:57jawsmanotejmeka: document.querySelector(&quot;#dialogFrame&quot;).contentWindow.document.documentElement gets me to the root element of the dialog
21:57MattNDo any of our tree walkers e.g. deep tree walker help with this?
21:58* MattN isn&#39;t too familiar but I believe it&#39;s what the devtools itself uses
21:58jawsMossop is also right above
21:58jawsMattN: mconley had suggested using a treewalker but it didn&#39;t traverse through boxObjects iirc
21:59MattNIs https://developer.mozilla.org/en-US/docs/Web/API/TreeWalker and @mozilla.org/inspector/deep-tree-walker;1 the same?
22:00manotejmekamattn,mossop: If I do browser.contentDocument then in the console I get something like &quot;Window chrome://browser/content/preferences/colors.xul&quot; How can I access the children of the content window. It says the children of it is null
22:00MattN#devtools can probably help with knowing whether a tree walker can do this for you. Or looking at the inspector code
22:00jawsMattN: document.children doesn&#39;t exist. you would need to use document.documentElement.children
22:00jawssorry, manotejmeka ^
22:01* jaws blames autocomplete
22:01MossopYeah so element.contentDocument.documentElement.children assuming element is the <browser> element there
22:01MattNmanotejmeka: here is the devtools code that I would recommend trying out: https://dxr.mozilla.org/mozilla-central/rev/6d38ad302429c98115c354d643e81987ecec5d3c/devtools/server/actors/inspector.js#2923-2957
22:02MattN&quot;showAnonymousContent&quot; and &quot;showSubDocuments&quot; are probably useful
22:03* MattN assumes this is for searching
22:03MossopAh, I was about to ask too
22:03MattNmanotejmeka: Are you implementing searching?
22:03manotejmekaSorry I am super confused. In the console I did &quot;$0.children[1].contentWindow&quot; which results in &quot; Window chrome://browser/content/preferences/colors.xul&quot;. $0 refers to &quot;<groupbox id=&quot;dialogBox&quot;....&quot;
22:03* Mossop wonders if those subdocuments are loaded on-demand or not
22:03manotejmekamattn: Yes I am implementing search to highlight the words I find
22:03MattNyou don&#39;t want contentWindow since you can get contentDocument instead
22:04MattNmanotejmeka: can we take a step back
22:04manotejmekaYes please
22:04MattNHow are you planning to implement search of subdialogs?
22:04MattNMy assumption was that we wouldn&#39;t load the contents of all dialogs
22:05MattNI think we should build an index manually or at build time rather than dynamically searching by loading all documents
22:05MattNe.g. a simple manual index would be adding an attribute e.g. @data-subdialog-text on some elements in preferences and it contains relevant keywords for the subdialog
22:06manotejmekaI already implemented search for normal XUL/HTML documents. What I do is I take a rootNode which in case would be &quot;<groupbox id=&quot;dialogBox&quot; orient=&quot;vertical&quot; pack=&quot;end&quot; role=&quot;dialog&quot; aria-labelledby=&quot;dialogTitle&quot; resizable=&quot;false&quot; style=&quot;min-width: calc(42px + 41em); min-height: calc(78px + 248px);&quot;>&quot; (Something similar to that). I check for it children and
22:06manotejmekago to the leaf nodes then I check the searched words with the contents of the leaf node if a match is found then I highlight.
22:06MattNthen we only have to filtering on that attribute instead of loading documents
22:06manotejmekaIn order for me to that I am getting blocked by the XULDocument Object because I can not walk that tree to find nodes with it to check for the content
22:06MattNok, I think that&#39;s fine for all the top-level panes that we already load eagerly
22:07MattNmanotejmeka: right, but don&#39;t we want to search the subdialogs even when they&#39;re not open?
22:07manotejmekaMattN: yes we need to search even if the subdialogs box is not open. So, what I though I can do is open it read it highlight it and close it.
22:08MattNok, I&#39;d advise against that approach
22:08MattNfor performance reasons
22:09MattNare you building an index once and then using it for every keypress?
22:09manotejmekaOkay in that case can you please suggest an idea on how I can tackle this problem.
22:09MattNI did above :)
22:09MattNtwo ideas
22:09MattNthough an answer to my last question would be good
22:10manotejmekaWhat do you mean by it would be nice to build an index at build time?
22:10manotejmekaCan you give me an example of what your thinking.
22:11MattNWhen we build Firefox we could figure out all of the strings in each XUL subdialog and store them in an object somewhere as an index
22:11MattNl10n repacks will be a problem probably
22:11MossopYeah localisation would make that a problem
22:11MattNI prefer my @data-subdialog-text solution
22:11MattNwhich wouldn&#39;t have that problem
22:12manotejmeka&quot;e.g. @data-subdialog-text on some elements in preferences and it contains relevant keywords for the subdialog&quot; I am little confused in this idea. Can you please elaborate.
22:12MattNsince you could refer to strings from that attribute
22:12MattNat least for .dtd ones
22:12* MattN tries to find a good example
22:12MossopMattN: Do you think it would be that expensive to load subdialogs async with fetch or something?
22:12manotejmekaAfter, I talked to jared and mike (jaws and mconley) we wanted to stay away from building an index at build time because there are many other complications with that idea
22:13MattNI think the value is low because many subdialogs don&#39;t contain more keywords than the button that opens them
22:13MossopFair enough
22:13MattNok, so suppose I want to change the colour of visited links
22:13MattNso I search for &quot;visited&quot;
22:14MattNnone of the panes contain &quot;visited&quot; so your current search excluding subdialogs won&#39;t find anything
22:14MattNthe correct answer is the <button xmlns=&quot;http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul&quot; id=&quot;colors&quot; icon=&quot;select-color&quot; label=&quot;Colors&quot; accesskey=&quot;C&quot;/>
22:14MattNthat&#39;s what I want to see I think?
22:14MattNor were we planning to show more than just that button for my use case?
22:15* MattN is assuming a certain UX and wants to check it
22:16MattN(we may want the section heading (&quot;Fonts & Colors&quot;) too, my question is about whether we were going to show/highlight new strings from the subdialog)
22:16MattNmanotejmeka: ^
22:16manotejmekaOne sec let me show you a small video so it will better explain things
22:17jawsMattN: we would put an arrow pointing at the button, and the section header would be shown too
22:18MattNMossop: I just realized you were talking about fetching the document (as an XML document to get entities probably), not rendering it which leads me to my next question
22:18jawsMattN: we will need to create a keyword list too for words that should be used but aren&#39;t actually on the page
22:18manotejmekahttps://youtu.be/7dVcj2PoubY?t=34 Here is an example of search that I have created so far.
22:18MattNjaws, manotejmeka: Do we want to consider dynamic content like domains in lists for permissions?
22:19jawsMattN: not yet, first step is to get the static text
22:19MattNok, they may need very different solutions
22:19manotejmekaIt finds all instances of the word searched for and displays. If the world were to be found in a dialogue box then the button that relates to that dialogue box would have a tooltip showing that the word is contained with this button
22:20MattNmanotejmeka: would the tooltip contain the string from the dialog?
22:21MattNi.e. for my example would it show &quot;Unvisited Link&quot; and &quot;Visited Links&quot; in the tooltip
22:21MattNif I search for &quot;visited&quot;
22:21manotejmekaYea for example in the picture I send you of the dialogue box If I were to search for the phrase &quot;Control which&quot; then the tooltip would be that
22:21manotejmekano it would only show visited
22:21MattNor simply a static string saying &quot;the dialog contains the text&quot;
22:21manotejmekaI would only highlight the wold that I am searching for or the phrase
22:22MattNok, so you want to highlight within open subdialogs?
22:22manotejmekaThe tooltip will contain the phrase/word that is searched for
22:22MattNok
22:22manotejmekaYes
22:22manotejmekaIf that world or phrase is found I want to highlight all instance of it
22:22MattNok, well in that case you need a solution for what you were working on anyways
22:23MattNso I would say to look into the inspector.js code I gave to use a deep tree walker
22:23manotejmekaYes
22:23MattNbut for a separate bug (IMO) to search for unloaded dialogs we can talk about other ideas
22:24MattNdoes that make sense?
22:25manotejmekaWhere can I find this inspector.js code.
22:25MattNI linked to it earlier
22:25MattNhttps://dxr.mozilla.org/mozilla-central/rev/6d38ad302429c98115c354d643e81987ecec5d3c/devtools/server/actors/inspector.js#2923-2957
22:26MattNI believe this is what the devtools inspector uses so it should do what you want
22:26manotejmekaAre you talking about this @mozilla.org/inspector/deep-tree-walker;
22:26MattNyes
22:26MattNYou will want to do something like most of the method
22:27manotejmekaLet me check the code and message you if I have any doubts about it. I think I sort of get the idea what your mentioning
22:27MattNDo you want to talk about ideas for unopened dialogs now still?
22:29manotejmekaYes please
22:30MattNok, if we don&#39;t care about any of the dynamic content in the dialogs, only what&#39;s in markup then I think Mossop&#39;s idea of doing an XHR/fetch sounds nice if it works with the XML parser
22:30manotejmekaThe thing is when the search is being done all dialogs will be unopened. The way we are implementing search. One can not search when a dialogue box is opened. As you can see from the image I posted in the beginning the search back is in the background on the about:preferences page.
22:31manotejmekaFor now I only want the markup.
22:31MattNoh, well I thought you were saying we would highlight in subdialogs
22:31manotejmekaI have no clue what this is &quot;XHR/fetch&quot;
22:32MattNso I guess ignore that tree walker stuff for now if your current solution is okay ignoring subdialogs
22:32manotejmekaMy first step is to find the content of the dialogue box. Then next step is to highlight those words in the dialogue box.
22:32MattNbut not render the dialog box?
22:32* MattN is confused
22:33manotejmekaHence in the beginning I mentioned that I was trying to tackle this problem by opening up the dialogue then traversing its nodes highlighting it and then closing it.
22:33MattNSo in the next step you will want to allow a search while the dialog box is open?
22:34MattNI&#39;m confused about &quot;highlight those words in the dialogue box&quot; since you said we won&#39;t do searches while it&#39;s open
22:34manotejmekaLet me clear the idea up a bit.
22:34MattNoh, so do you mean that you will show the matching text for the previous search
22:34MattNi.e. clicking a button to open a subdialog doesn&#39;t clear the highlights?
22:34MattNThis would be a lot easier with a UX spec :)
22:36manotejmekaThe search bar is in about:preferences page in all the panes that are available there. When one searched for a key words like &quot;Visited&quot; I go through the entire about:preferences mark up to find and highlight the words (This is already done as shown in the video). What I want to add to it is when buttons are linked to popup dialogs I want to find the content
22:36manotejmekawith in those dialogues and highlight them. If one popup consists of the world being searched for then for the button associated with it I will have a tooltip showing the word searched for
22:37manotejmekamattn: So you can see how there is a tooltip above the settings button like that https://usercontent.irccloud-cdn.com/file/JvALG6XS/Screen%20Shot%202017-03-14%20at%206.36.38%20PM.png
22:37MattNI see
22:38MattNand if I click on a button to open a subdialog the subdialog should also have &quot;co&quot; highlighted within text?
22:38manotejmekaWhat this mean is that the phrase &quot;co&quot; is found with in the subdialogue box that is accessed through that button. So if one clicks on that button to open the dialogue The phrase &quot;co&quot; would be highligted with in
22:38manotejmekaYes
22:38MattNok
22:38manotejmekaSorry for my typos and confusion
22:39MattNno problem, it&#39;s complicated
22:39manotejmekaWhen the search is cleared then all the tooltips and highlighting will be removed. Or when a new search is done &quot;co&quot; phrase highlights will all be removed as well
22:39MattNok
22:41manotejmekamattn: I might need to leave soon. Please message me personally about any new ideas you have. I will be texting you from my IRC phone app from now. I need to switch classes :)
22:42MattNmanotejmeka: ok
22:43MattNmanotejmeka: Will you get scrollback when moving between classes?
22:43MattNI haven&#39;t used Fetch enough so I will explain with XHR. XHR = https://developer.mozilla.org/en/docs/Web/API/XMLHttpRequest
22:44manotejmekaYes I will I bought the IRC so it is always active and I have it on my phone as well so I am connected always
22:44MattNok, I will continue the discussion here and you can reply when available
22:44manotejmekaThank you so much for your help.
22:44manotejmekaMattN: ^
22:45MattNyou&#39;re welcome
22:46MattNBasically the idea is to have a list of subdialog (XUL) URLs (e.g. colors.xul) and upon loading about:preferences you can request parallel request to get the contents from all of them with XHR
22:46MattNYou don&#39;t just want the responseText string, you need it to be parsed with the XML parser so you can set https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/responseType to &quot;document&quot;
22:47MattNNote that this approach won&#39;t with with strings in .properties files
22:47MattN*won&#39;t work with
22:48MattNFrom those requests you can extract the text like you currently do since you would have a XULDocument to work with
22:48MattNThen you can keep a mapping of the strings to the XUL files and the associated subdialog buttons
22:49MattNFor string in .properties files I think we can either a) move some to .dtd and/or b) add keywords for them using the same mechanism we&#39;ll use for other aliases/keywords which aren&#39;t in strings
22:50MossopMy only concern would be that I don&#39;t think XBL bindings are attached for that kind of load so if you&#39;re expecting strings that come from XBL you&#39;d be out of luck
22:51MattNCan XBL bindings use .dtd strings?
22:51MossopSure, in the anonymous content
22:51MossopI don&#39;t think many do though so in reality it may not be a problem\
22:52MattNsame problem and solution as .properties that I mentioned too
22:52MossopTrue\
22:52MattNProposal 2: Don&#39;t do XHR/fetch and manually define keywords on <button> for each of the subdialogs in .dtd files. That way you don&#39;t need to request all the XUL files. You can even refer to existing .dtd entities from the attribute containing the keywords
22:54MattNe.g. <button id=&quot;colors&quot; icon=&quot;select-color&quot; label=&quot;Colors&quot; accesskey=&quot;C&quot; data-subdialog-keywords=&quot;&visitedLinkColor.label; &linkColor.label;&quot;/>
22:54MattNThen you can have an automated mochitest browser-chrome test to enforce new keywords are added (at least for English)
22:55MattNthe test could load the dialogs and ensure that new keywords aren&#39;t introduced in the dialog but no