Bug 1739934 Comment 16 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

Original comment by
on 
(In reply to Sebastian Kaspari (:sebastian; :pocmo) from comment #14)
> Comment on attachment 9250811 [details]
> AC.patch
> 
> Unfortunately Bugzilla doesn't show the "review"/Splinter option here?
> 
> The patch looks good.
> 
> nit: I think we should move the `getFlags()` method from `EngineSession` (concept) to `GeckoEngineSession`, maybe as an extension method. Stripping this specific flag before passing it along is a Gecko-specific thing we do, so let's do it only internally in `GeckoEngineSession` (We could also try naming it in a way that it is more clear that we are filtering the flags down to Gecko supported flags.

Got it, no problem.
Revision 1 by
on 
(In reply to Sebastian Kaspari (:sebastian; :pocmo) from comment #14)
> Comment on attachment 9250811 [details]
> AC.patch
> 
> Unfortunately Bugzilla doesn't show the "review"/Splinter option here?
> 
> The patch looks good.
> 
> nit: I think we should move the `getFlags()` method from `EngineSession` (concept) to `GeckoEngineSession`, maybe as an extension method. Stripping this specific flag before passing it along is a Gecko-specific thing we do, so let's do it only internally in `GeckoEngineSession` (We could also try naming it in a way that it is more clear that we are filtering the flags down to Gecko supported flags.

Thanks for the review.
Got it, no problem.

Back to Bug 1739934 Comment 16