On 2017-04-04 7:55 AM, Aryeh Gregor wrote: > On Mon, Apr 3, 2017 at 11:39 PM, Benjamin Smedberg > <benja...@smedbergs.us> wrote: >> >> I'd like to encourage you to set up a test plan for this. My impression of >> the risk profile of this work is that we could easily break some really >> important use-cases, and it's likely that sites customize for gecko behavior >> and rely on it, either accidentally or on purpose. This is definitely the >> kind of thing that would be worth rolling out carefully and perhaps slowly. >> Will this behavior be behind a pref which is easy to flip, test, and roll >> out? > > As implemented, it is not behind a pref. Masayuki didn't request a > gradual rollout, so I pushed to inbound already. I suspect it will > not actually break anything, because sites that use the editor > normally avoid browser compatibility issues here by completely > overriding the newline behavior, so they will be unaffected. If any > sites actually do break from this, I would be very interested to have > the data that people are really using our default newline behavior! > > I'd like to know what Ehsan and Masayuki think about the likely compat > impact here.
I think this is a very risky change. While some editor widgets may already be UA sniffing Gecko and using the defaultParagraphSeparator command already, others may be relying on us injecting a <br> by default and be fixing up the DOM after the fact, and those widgets could potentially break in all kinds of potentially surprising ways as a result of this change. Unfortunately, historically many of the bugs like this only get reported to us very late in the beta cycle (or even after the change gets to the release channel.) I don't own this module any more, so this isn't my call to make, but if I had to choose what to do here, I would probably either choose to not change our behavior (since I'm not sure what we're gaining here concretely -- as AFAIK we're not investing in bringing our behavior on par with other engines on a more broad basis with regards to editing), or at the lack of that, adding some telemetry first to get data on how often the defaultParagraphSeparator command is used in the wild, since AFAICT your change is basically only Web compatible on the assumption that this command is used quite heavily. I'll also note that detecting what breaks here isn't even easy when the defaultParagraphSeparator command has not been executed, so the telemetry data indicating where that command has _not_ been executed can't necessarily account for the cases where your change could cause potential issues. On the idea of the test plan that Benjamin brought up, I'm not sure what to put in such a test plan, due to the issue I mentioned above (it being totally non-obvious what the expected breakage of this change would look like.) Cheers, Ehsan _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
