On 2017-04-05 7:27 AM, Aryeh Gregor wrote: > On Wed, Apr 5, 2017 at 3:12 AM, Ehsan Akhgari <ehsan.akhg...@gmail.com> wrote: >> Exactly. We can hypothesize either way, but we certainly can't know easily >> without getting some data first. But unfortunately it's not possible to >> collect data about what sites are doing in terms of DOM fix-ups like this. >> We can, at least, collect data about whether they are overriding the newline >> behavior wholesale though. Is there any reason why we should not at least >> first collect this data before changing the behavior here? > > What exact data would you want? We could collect data on how often > our built-in newline behavior is used, and if it's low enough we'd be > confident the change is safe.
Originally it seemed that you are working under the assumption that most sites are overriding our default newline handling behavior. This is very easy to measure through telemetry by adding a probe for example that looks when an HTML editor object is destroyed whether the defaultParagraphSeparator command was used to override our default behavior, you can send a value of 0 for the false case and a value of 1 for the true case and we can get a percentage of pages where this override actually happens on based on this data. > If it's not so low, however, we'd need > to look at the actual sites to see if they break. Can we do that > somehow through telemetry, or is it a privacy problem? Detecting the actual breakage that can happen is much more difficult in code because you would need to first define what the breakage would result in to try to detect it in code which is extremely difficult in this case. > If someone has suggestions for the exact telemetry probe to use here, > I'm happy to add one, and maybe keep the change in Aurora until we get > data to make a decision. I'm not familiar enough with telemetry > (either the theoretical options or our implementation) to decide what > the right probe is. Getting the kind of data I'm suggesting above _now_ that the patch is landed seems rather pointless. There seems to be a lot of disagreement on the degree of the risk involved in this change in the first place, and until we agree on the level of risk arguing about the details like this may be pointless. At the end of the day, this is Masayuki's call. I certainly understand that in the past with changes like this we've had a lot of difficulty detecting regressions before the patches have hit the release channel, so I'm not sure how much keeping the patch on pre-release channels would really help. :-( But to me it seems like the kind of thing that we'd want to be able to quickly turn off on the release channel through shipping a hotfix add-on that sets a pref if something goes wrong... > On Wed, Apr 5, 2017 at 3:31 AM, Benjamin Smedberg <benja...@smedbergs.us> > wrote: >> I agree that it doesn't seem likely that telemetry can answer this sort of >> question. However, we could collect data! Pick N top editing tools and >> actually test their behavior. We probably can't get full confidence, but we >> can get a much better picture of the risk involved. If we break (or >> significantly change behavior) on five sites out of 40, that's a strong >> indicator that we're going to have problems. >> >> As an example, have we already tested or is it in a plan to test: >> google docs >> basic and rich text editors on wikipedia >> office 365 >> github editor >> common rich text editor libraries, and common CRM software (I don't have a >> list) >> the top hosted email sites: gmail, yahoo, hotmail/outlook, aol, icloud, >> yandex >> >> Being able to assert, before landing this change, that it doesn't break any >> of these sites, would really help in terms of making assertions about the >> risk profile. > > I did not test this specific change on those sites. However, some > years ago I did research execCommand() usage on the web, and found > that high-profile richtext editors essentially didn't use it, because > of browser incompatibilities. Instead, they wrote their own > functions. The same incompatibilities exist in newline behavior, so > it's reasonable to say that they would write their own functions for > that too. > > This is also supported by a discussion I had with a couple of > developers of major richtext editing libraries (I don't remember which > offhand). They said that changes like this make no difference to > them, because they don't use the functionality anyway. They're > interested in fixes in things like selection handling, which they do > use, and features that allow them to more easily override browser > behavior. > > Also anecdotally, in terms of dealing with editor bugs like this -- > the reports most often come from Thunderbird. Maybe Ehsan or Masayuki > could weigh in too, but I think that we get very few bug reports in > these parts of the editor from real-world websites. (We do get some > reports in other parts of the editor, like selection/focus handling.) > This also suggests websites aren't actually using this code much. > (Although maybe it means they just work around our bugs already.) > > In fact, I dropped this patch set for a while because the feature is > seldom-used enough on the web that it doesn't seem worth fixing. I > get the impression that Ehsan shares this point of view. Perhaps I'm missing something about the nature of what changed here. How is this seldom used? Am I misunderstanding that the change happened was how *Gecko* reacts to the user pressing Enter by default in a contenteditable section? It's true that some editing widgets override this behavior somehow, but I'd be really shocked if that's true 100% of the time, so I don't understand how you argue this is a seldom used feature... > It's also worth noting that contenteditable is a very complicated > feature to use in real life, particularly given browser > incompatibilities, and I think almost all sites that use it are either > very large or use one of the major rich-text editing libraries. If > I'm correct, then we don't have to worry so much about breaking a long > tail of small sites that won't get reported quickly. If Gmail, > Wikipedia, TinyMCE, etc. break, we (or they) are likely to get reports > soon enough. Large sites are also usually well-maintained and do > their own testing, and will fix the issue quickly. (But if a library > breaks, or software like MediaWiki, there will be a long tail of old > sites that will remain broken even after the library is fixed, because > they keep using old versions of the library.) > > So that's my reasoning for why I don't think this is *so* risky. But > I agree that I don't really know. As I said, I'd be happy to let this > stay on Aurora or beta for longer, and/or add some telemetry (if > someone tells me what telemetry we want). I guess I'm personally coming from the perspective of having bitten by regressing various things about the editor behavior too many times in the past, and every time it happened, we could make an argument about how it's extremely unlikely to happen beforehand. :-) In my experience, regressions affecting this code are usually of two kinds, they either are severe enough that they break *many* sites and you'll get a few bug reports on the Nightly channel after a few days, or you basically don't hear about them at all until late during the beta cycle or when the patch hits the release channel. Manually testing sites and libraries, while useful, only covers a tiny surface of what's needed, since a lot of the editor libraries have lots of old versions that are used in the wild and they all have their own behaviors and they rarely get updated, so it's not just a matter of testing the latest version of a few libraries and sites. And often times the older and smaller websites tend to use more of the browser functionality and the larger sites tend to implement more of their own functionality on top of the raw DOM APIs which creates an unhelpful dynamic where the sites that are less likely to be tested in our pre-release channels are more likely to break by these changes, etc. :-/ Anyways, I hope this is all helpful. Cheers, Ehsan _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
