These have started to appear in the last few weeks.

Example crash stack: bp-58efffa3-badf-406a-b135-97abe0210704

            0  libsystem_platform.dylib  _platform_memmove$VARIANT$Haswell   context
            1   @0x7ffee99ec2df   cfi
            2  XUL  _cairo_surface_to_cgimage  gfx/cairo/cairo/src/cairo-quartz-surface.c:870  frame_pointer
            3  XUL  _cairo_quartz_setup_state  gfx/cairo/cairo/src/cairo-quartz-surface.c:1353  frame_pointer
            4  XUL  _cairo_quartz_cg_fill  gfx/cairo/cairo/src/cairo-quartz-surface.c:1935  frame_pointer
            5  XUL  _cairo_compositor_fill  gfx/cairo/cairo/src/cairo-compositor.c:203  frame_pointer
            6  XUL  _cairo_quartz_surface_fill  gfx/cairo/cairo/src/cairo-quartz-surface.c:2237  frame_pointer
            7  XUL  _cairo_surface_fill  gfx/cairo/cairo/src/cairo-surface.c:2473  frame_pointer
            8  XUL  _cairo_gstate_fill  gfx/cairo/cairo/src/cairo-gstate.c:1312  frame_pointer
            9  XUL  _moz_cairo_fill_preserve  gfx/cairo/cairo/src/cairo.c:2447  frame_pointer
            10  XUL  mozilla::gfx::DrawTargetCairo::DrawPattern(mozilla::gfx::Pattern const&, mozilla::gfx::StrokeOptions const&, mozilla::gfx::DrawOptions const&, mozilla::gfx::DrawTargetCairo::DrawPatternType, bool)  gfx/2d/DrawTargetCairo.cpp:1009  frame_pointer
            11  XUL  mozilla::gfx::DrawTargetCairo::FillRect(mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits, float> const&, mozilla::gfx::Pattern const&, mozilla::gfx::DrawOptions const&)  gfx/2d/DrawTargetCairo.cpp:1059  frame_pointer
            12  XUL  mozilla::gfx::RecordedFillRect::PlayEvent(mozilla::gfx::Translator*) const  gfx/2d/RecordedEventImpl.h:2229  frame_pointer
            13  XUL  bool mozilla::gfx::RecordedEvent::DoWithEvent<mozilla::gfx::EventStream>(mozilla::gfx::EventStream&, mozilla::gfx::RecordedEvent::EventType, std::__1::function<bool (mozilla::gfx::RecordedEvent*)> const&)  gfx/2d/RecordedEventImpl.h:3989  frame_pointer
            14  XUL  mozilla::layout::PrintTranslator::TranslateRecording(mozilla::layout::PRFileDescStream&)  layout/printing/PrintTranslator.cpp:50  frame_pointer
            15  XUL  mozilla::layout::RemotePrintJobParent::PrintPage(mozilla::layout::PRFileDescStream&, nsRefCountedHashtable<nsUint64HashKey, RefPtr<mozilla::gfx::RecordedDependentSurface> >*)  layout/printing/ipc/RemotePrintJobParent.cpp:167  frame_pointer
            16  XUL  mozilla::layout::RemotePrintJobParent::FinishProcessingPage(nsRefCountedHashtable<nsUint64HashKey, RefPtr<mozilla::gfx::RecordedDependentSurface> >*)  layout/printing/ipc/RemotePrintJobParent.cpp:146  frame_pointer
            17  XUL  mozilla::layout::RemotePrintJobParent::RecvProcessPage(nsTArray<unsigned long long>&&)  layout/printing/ipc/RemotePrintJobParent.cpp:121  frame_pointer
            18  XUL  mozilla::layout::PRemotePrintJobParent::OnMessageReceived(IPC::Message const&)  ipc/ipdl/PRemotePrintJobParent.cpp:301  frame_pointer
            19  XUL  mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&)  ipc/ipdl/PContentParent.cpp:6677  frame_pointer
            20  XUL  mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&)  ipc/glue/MessageChannel.cpp:2079  frame_pointer
            21  XUL  mozilla::ipc::MessageChannel::MessageTask::Run()  ipc/glue/MessageChannel.cpp:1955  frame_pointer
            22  XUL  mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&)  xpcom/threads/TaskController.cpp:785  frame_pointer
            23  XUL  mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_3>::Run()  xpcom/threads/nsThreadUtils.h:534  frame_pointer
            24  XUL  nsThread::ProcessNextEvent(bool, bool*)  xpcom/threads/nsThread.cpp:1155  frame_pointer
            25  XUL  NS_ProcessPendingEvents(nsIThread*, unsigned int)  xpcom/threads/nsThreadUtils.cpp:496  frame_pointer
            26  XUL  nsAppShell::ProcessGeckoEvents(void*)  widget/cocoa/nsAppShell.mm:467  frame_pointer
            27  CoreFoundation  __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__   frame_pointer
            28  CoreFoundation  __CFRunLoopDoSource0   cfi
            29  CoreFoundation  __CFRunLoopDoSources0   cfi
            30  CoreFoundation  __CFRunLoopRun   cfi
            31  CoreFoundation  CFRunLoopRunSpecific   cfi
            32  HIToolbox  RunCurrentEventLoopInMode   cfi
            33  HIToolbox  ReceiveNextEventCommon   cfi
            34  HIToolbox  _BlockUntilNextEventMatchingListInModeWithFilter   cfi
            35  AppKit  _DPSNextEvent   cfi
            36  AppKit  -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:]   cfi
            37  XUL  -[GeckoNSApplication nextEventMatchingMask:untilDate:inMode:dequeue:]  widget/cocoa/nsAppShell.mm:170  cfi
            38  AppKit  -[NSApplication run]   frame_pointer
            39  XUL  nsAppShell::Run()  widget/cocoa/nsAppShell.mm:732  cfi
            40  XUL  nsAppStartup::Run()  toolkit/components/startup/nsAppStartup.cpp:273  frame_pointer
            41  XUL  XRE_main(int, char**, mozilla::BootstrapConfig const&)  toolkit/xre/nsAppRunner.cpp:5496  frame_pointer
            42  firefox  main  browser/app/nsBrowserApp.cpp:351  frame_pointer
            43  libdyld.dylib  start   frame_pointer
            44  libdyld.dylib  start   cfi

Most, if not all, of these have to do with printing -- they have mozilla::layout::PrintTranslator::... on the stack.

Seems like this would most plausibly be related to the cairo update (bug 739096), but that landed at the end of April so it's odd that the crashes don't start showing up until June. (And why only on Beta, no Nightly reports?)

For what it's worth, the cairo update landed on the 90 branch, and the 90 branch reached beta on 2021/06/01 -- which is around the time these crashes started appearing. I suspect that some features, like the print dialog, don't get used much except on beta and release versions. That could explain the absence of crashes on trunk.

Here are the top few lines (in X86_64 assembly language) of the platform_memmove() function at the top of this bug's stacks. The crashes all seem to happen at the last line (rep movsb byte [rdi], byte [rsi]). So a fairly large number of bytes are being copied (> 0x4000). The crash addresses are always what's in the rsi register, which points to image_surface->data. But the value in the rcx register is usually quite small, so the bad accesses happen towards the end of the operation. These could be buffer overflows.

                         __platform_memmove$VARIANT$Haswell:
    0000000000000900         push       rbp
    0000000000000901         mov        rbp, rsp
    0000000000000904         mov        r11, rdi
    0000000000000907         sub        r11, rsi
    000000000000090a         mov        rax, rdi
    000000000000090d         cmp        r11, rdx
    0000000000000910         jb         loc_92d
    
    0000000000000912         cmp        rdx, 0x60
    0000000000000916         jbe        loc_947
    
    0000000000000918         cmp        rdx, 0x4000
    000000000000091f         jb         loc_9f0
    
    0000000000000925         mov        rcx, rdx
    0000000000000928         cld
    0000000000000929         rep movsb  byte [rdi], byte [rsi]
    ...

Hi,

I was able to reproduce this crash using the attached minimal HTML in Firefox Nightly 96.0a1 (2021-11-17) (64-bit) on macOS 11.

Steps to reproduce:

• Open the html file
• Try to print
• Select Margin: None in print dialog
• Select "Print Backgrounds"

The browser will crash. I was able to reproduce this 100% of the time.

(In reply to Gopi Krishna from comment #5)

Created attachment 9251328 [details]
index.html

Hi,

I was able to reproduce this crash using the attached minimal HTML in Firefox Nightly 96.0a1 (2021-11-17) (64-bit) on macOS 11.

Steps to reproduce:

• Open the html file
• Try to print
• Select Margin: None in print dialog
• Select "Print Backgrounds"

The browser will crash. I was able to reproduce this 100% of the time.

Sorry, missed adding this.

Try to print after selecting "Print Backgrounds"

Thanks, Gopi! Your test case works!

I tried it in FF 94.0.1 on all major versions of macOS from 12 back to 10.12. It crashes on all of them. But for some reason it takes much longer on macOS 10.12 (it seems the background images take much longer to "prepare" there).

The only full crash stacks I see are on macOS 11 and 10.13 and 10.12. On other versions of macOS, the stack is truncated to the top line (_platform_memmove$VARIANT$Haswell).

At some point (and if nobody else beats me to it) I'll use HookCase to figure out what's going on. But it'll be a while before I have the time for that.

Here's the first mozilla-central nightly these crashes happen on. It's where the cairo update first landed (bug 739096).

http://ftp.mozilla.org/pub/firefox/nightly/2021/04/2021-04-29-21-42-31-mozilla-central/firefox-90.0a1.en-US.mac.dmg

They don't happen with the one just previous:

http://ftp.mozilla.org/pub/firefox/nightly/2021/04/2021-04-29-09-26-05-mozilla-central/firefox-90.0a1.en-US.mac.dmg

Set release status flags based on info from the regressing bug 739096

I've also encountered this crash with macOS 11 on beta 95.0b11 using the following steps:

1. Go to https://www.ebay.com/n/error
2. Select the “Go to homepage” button
3. Right click on the button and choose “Print Selection”
4. Attempt to Save the page to PDF

I've started working on this, and I've learned a few things. But I've discovered something very puzzling, which I thought I should mention.

The crashes happen here. image_surface (of type cairo_image_surface_t) has been "acquired" above. Now the code has allocated image_data and is trying to copy the contents of image_surface->data to it.

The allocation doesn't fail, and is the correct size. I haven't yet determined whether the amount of data being copied is correct -- whether image_surface->data might contain less data than expected. Since the crash address is the value in the rsi register (i.e. image_surface->data), this seems likely.

But along the way I've discovered that, when a cairo_image_surface_t object is being initialized (by _cairo_image_surface_init()), all its fields are filled with the value (kAllocPoison == 0xe5) jemalloc uses to poison freed memory. Since this object is newly allocated (here), it should have been filled with the value (kAllocJunk == 0xe4) that jemalloc uses to poison newly allocated memory.

Does anyone know what's going on?

I just found out that disabling jemalloc in a local build (ac_add_options --disable-jemalloc) "fixes" this bug. By itself, that doesn't mean it's caused by jemalloc. I'll keep looking for other, less disturbing possible causes. But this may turn out to be a jemalloc bug.

I've got what I think is a fix for this. It's quite simple:

    diff --git a/gfx/2d/RecordedEventImpl.h b/gfx/2d/RecordedEventImpl.h
    --- a/gfx/2d/RecordedEventImpl.h
    +++ b/gfx/2d/RecordedEventImpl.h
    @@ -3082,15 +3082,28 @@ RecordedSourceSurfaceCreation::RecordedS
         return;
       }
     
    -  size_t size = 0;
    +  // Add an extra line to the allocation size. This may be needed when
    +  // creating a subimage of this surface, for example in
    +  // mozilla::gfx::CreateSubImageForData(). When the subimage will have
    +  // non-zero X or Y coordinates (with respect to the parent image), the data
    +  // passed to the method that creates the subimage will be at an offset in
    +  // mData. We need to reduce the expected size of the subimage by at least
    +  // this amount. But a non-zero X coordinate (in effect) increases the
    +  // subimage's expected size without compensation: An image's size is always
    +  // assumed to be height * stride, but a non-zero X coordinate doesn't change
    +  // the subimage's height. The missing compensation is never greater than the
    +  // size of one line (i.e. the original image's stride). So the most we'll
    +  // ever need is one extra line. This change fixes bug 1719215.
    +  size_t size = 0, alloc_size = 0;
       if (mSize.width >= 0 && mSize.height >= 0) {
         size = size_t(mSize.width) * size_t(mSize.height) * BytesPerPixel(mFormat);
    -    mData = new (fallible) uint8_t[size];
    +    alloc_size = size_t(mSize.width) * size_t(mSize.height + 1) * BytesPerPixel(mFormat);
    +    mData = new (fallible) uint8_t[alloc_size];
       }
       if (!mData) {
         gfxCriticalNote
             << "RecordedSourceSurfaceCreation failed to allocate data of size "
    -        << size;
    +        << alloc_size;
         aStream.SetIsBad();
       } else {
         aStream.read((char*)mData, size);

I tried to do a tryserver build, but I no longer have access. I'll try to get my access restored, and then do one.

Same patch, revised comment:

    diff --git a/gfx/2d/RecordedEventImpl.h b/gfx/2d/RecordedEventImpl.h
    --- a/gfx/2d/RecordedEventImpl.h
    +++ b/gfx/2d/RecordedEventImpl.h
    @@ -3082,15 +3082,30 @@ RecordedSourceSurfaceCreation::RecordedS
         return;
       }
     
    -  size_t size = 0;
    +  // Add an extra line to the allocation size. This may be needed when
    +  // creating a subimage of this surface, for example in
    +  // mozilla::gfx::CreateSubImageForData(). When the subimage has non-zero
    +  // X or Y coordinates (with respect to the parent image), the data passed to
    +  // the method that creates the subimage will be at an offset in mData. We
    +  // need to reduce the expected size of the subimage by this amount. But a
    +  // non-zero X coordinate doesn't do this: An image's size is always assumed
    +  // to be height * stride, but a non-zero X coordinate doesn't change the
    +  // subimage's height. This lack of compensation can cause a subimage's
    +  // expected size (from the offset in mData) to be greater than its actual
    +  // size, leading to crashes due to access failures. The missing compensation
    +  // is never greater than the size of one line (i.e. the original image's
    +  // stride). So the most we'll ever need is one extra line. This change fixes
    +  // bug 1719215.
    +  size_t size = 0, alloc_size = 0;
       if (mSize.width >= 0 && mSize.height >= 0) {
         size = size_t(mSize.width) * size_t(mSize.height) * BytesPerPixel(mFormat);
    -    mData = new (fallible) uint8_t[size];
    +    alloc_size = size_t(mSize.width) * size_t(mSize.height + 1) * BytesPerPixel(mFormat);
    +    mData = new (fallible) uint8_t[alloc_size];
       }
       if (!mData) {
         gfxCriticalNote
             << "RecordedSourceSurfaceCreation failed to allocate data of size "
    -        << size;
    +        << alloc_size;
         aStream.SetIsBad();
       } else {
         aStream.read((char*)mData, size);

👍👍

My account's been reactivated, and I pushed a build to try:

https://treeherder.mozilla.org/jobs?repo=try&revision=5bbd12681a2b76e7e8ec8ad9995905f48a957683

I'll look at the test results as they become available. I'll also play around with the symbolized universal build (if I've used the correct incantation to get it created).

I'll also play around with the symbolized universal build

Here it is: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/eKKu4txNSYGJwHH1nm3t5A/runs/1/artifacts/public/build/target.dmg

Note that you must run xattr -c target.dmg on it (from a Terminal prompt) before double-clicking on it to extract its contents. Otherwise the Firefox Nightly build you extract will be unusable.

Those who can reproduce these crashes, please try it and let us know your results.

I may need to revise my patch. For large enough values of aRect.X() in CreateSubImageForData(), I still see this bug's crashes, even with my patch. One extra line of allocation size may not be enough.

I don't yet fully understand how that code works -- particularly the clipping (if that's what it is) performed by aRect. I need to figure it out.

Hi Steven,

I have tried the target.dmg build, It seems to work fine, not crashing anymore.

I don't yet fully understand how that code works -- particularly the clipping (if that's what it is) performed by aRect. I need to figure it out.

I've been working on this, intermittently, for the last few weeks. I'm still not finished -- the code I'm looking at is fiercely complex and scattered through the Mozilla tree. And now I'll have to put off further work until after the holiday season (until early next year).

Here's my current patch: https://hg.mozilla.org/try/rev/188a5f2afeb81f97e49cd431c716d63d0498ba4f

And here's a tryserver build made from it: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/P1dOizdhT0K6rwJbxH3otQ/runs/0/artifacts/public/build/target.dmg

Please try it out, Gopi, and let us know your results.

Note once again that you must run xattr -c target.dmg on it (from a Terminal prompt) before double-clicking on it to extract its contents. Otherwise the Firefox Nightly build you extract will be unusable.

Hi Steven,

sorry about the delay, have been busy. I have tried the https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/P1dOizdhT0K6rwJbxH3otQ/runs/0/artifacts/public/build/target.dmg build, It seems to work fine, not crashing anymore.

Hi Steven, given comment 22, did you want to put your patch up for review?

I haven't done any more work on this bug since comment #21.

My patch seems to work, and it makes sense to me in at least a limited fashion. But I have the nagging feeling that the real problem lies elsewhere, or at least that I don't fully understand why my patch works. So I'm not yet ready to ask for review.

I've been very busy with other things. But I think I can put them aside for the next week or so, in an attempt to finally put this bug to rest.

7 crashes in the last week, downgrading this.

I've banged my head against this some more over the last week. I didn't find a better fix, but I've rewritten my code comments to make them clearer (I hope). And I'm about to submit my patch for review.

As I mentioned above, the crashes happen here. image_surface (of type cairo_image_surface_t) has been "acquired" above. Now the code has allocated image_data and is trying to copy the contents of image_surface->data to it.

The problem is that image_surface->data sometimes doesn't point to the original image data, but to an offset in it which is in the middle of a "line" in the original image bitmap data. When this happens, image_surface->height * image_surface->stride + offset is slightly longer than the length of the data which should be copied. This can lead to buffer overflows, which can trigger this bug's crashes.

The code that (ultimately) causes this problem is here. It happens when aRect.X() is non-zero.

It took me a long time to figure out why, but + aRect.X() * BytesPerPixel(aFormat) is completely unnecessary. So the fix is to get rid of this code snippet.

I've started a tryserver build to test my patch:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=cf7c062ab7bdc6b7d4d7e387703eb77f212f2371

https://hg.mozilla.org/mozilla-central/rev/d8b66c3db775

Here's the first mozilla-central nightly that contains this bug's fix:

https://ftp.mozilla.org/pub/firefox/nightly/2022/02/2022-02-04-09-29-58-mozilla-central/firefox-98.0a1.en-US.mac.dmg

Whoever sees this bug's crashes, please try it out.

I can no longer reproduce this crash (using steps from comment 6 and comment 10); tested with the latest Nightly 98.0a1 (2022-02-07) on macOS 10.13, 11.0 and 12.0.

Did you want to nominate this for ESR uplift? It grafts cleanly as-landed.

Comment on attachment 9262151 [details]
Bug 1719215 - Constrain clipping in CreateSubImageForData(). r=lsalzman

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This patch fixes a fairly high-frequency macOS crash.
• User impact if declined: The crashes will keep happening on the ESR branch.
• Fix Landed on Version: 98
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The patch has been baking on the trunk for a little more than two weeks, and on beta for almost as long. No problems reported.

Comment on attachment 9262151 [details]
Bug 1719215 - Constrain clipping in CreateSubImageForData(). r=lsalzman

Approved for 91.7esr.

https://hg.mozilla.org/releases/mozilla-esr91/rev/63bf31358d0e

This crash is no longer reproducible (using steps from comment 6 and comment 10) with the latest Esr build (91.7.0esr-build2) on macOS 10.13, 11.0 and 12.0.

This bug's patch has eliminated crashes with signatures containing _cairo_surface_to_cgimage. We're still seeing crashes with signatures containing _platform_memmove, but those are unrelated to the original bug (not involving printing, and not on the main thread):

https://crash-stats.mozilla.org/search/?signature=~_platform_memmove&version=98.0&version=98.0.1&version=98.0.2&version=91.7.0esr&version=91.7.1esr&platform=Mac%20OS%20X&date=%3E%3D2021-12-25T15%3A59%3A00.000Z&date=%3C2022-03-25T15%3A59%3A00.000Z&_facets=signature&_facets=proto_signature&_sort=-date&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#facet-proto_signature

So yes, this bug does seem to have been fixed.

From looking at the regression in bug 1769429, I don't this change was correct. cairo_surface_set_device_offset won't change where data is read from in the buffer. With this change, trying to a subset an image with non-zero aRect.x will not properly take aRect.x into account and will always start reading data from x = 0.

I believe we've run into this problem before where CoreGraphics can read up to height * stride instead of clamping at (height-1)*stride + width*bpp. I don't have an immediate suggestion for how to fix it.

I'm going to leave this in your hands, Jeff. Or someone else's.

I spent many hours on this bug, and couldn't come up with a better solution.

IMO, unless we're planning to back out the patch that landed here, we should leave this bug closed and track the follow-up work in bug 1769429.

It seems like we could change the memcpy here: https://searchfox.org/mozilla-central/rev/bf6f194694c9d1ae92847f3d4e4c15c2486f3200/gfx/cairo/cairo/src/cairo-quartz-image-surface.c#349 to (height - 1) * stride + cairo_format_stride_for_width (format, width). Something like https://hg.mozilla.org/mozilla-central/rev/accdbdfaa6e0

That would presumably resolve this bug's crashes. But if you backed out my original patch, you'd have to find some other way to deal with the problem outlined in my comment for it. Specifically:

"Here we ensure that no clipping is done in the X dimension at the beginning of any line. (To do otherwise would require creating a copy of aData from parts of every line in aData (from aRect.Y() to aRect.Height()), and setting the copy to a different stride.)"

I don't understand that comment. Can you elaborate on why a copy is needed?

OK. I constructed a simple example to show what I meant by that comment ... and I now realize it's wrong :-( Your proposed patch, Jeff, is all that's needed here. My original patch can be backed out. Though please do leave the // Set the subimage's location in its parent comment in place. It makes the original code's logic a lot clearer.

Say you have image data for an image. The image's height (in pixels) is height and its width is width. Each line of data has bytes_per_pixel * width bytes. To keep things simple, let's say the stride for the whole image is also bytes_per_pixel * width. The image's data is stored in a single, continuous block starting at image_data.

Let's say you want to get the data corresponding to a subimage of the original image, where the subimage's subimage_x and subimage_y coordinates are offsets in height and width. (Let's also say that the subimage is entirely contained in the original image.)

If subimage_x is 0, it's very straightforward. The subimage's data is also a continuous block starting at image_data + subimage_y * image_stride.

But if subimage_x is (say) 1, its data is no longer in a continuous block inside image_data. However (and this is where I was wrong) the subimage's stride can stay the same as that of the original image. When you copy data for the subimage starting at aData + aRect.Y() * aStride + aRect.X() * BytesPerPixel(aFormat), the copy will include some data not in the subimage. But as long as the code which displays the subimage knows to skip the first 1 * bytes_per_pixel (aka aRect.X() * BytesPerPixel(aFormat)) bytes of each line, it will be able to do so correctly. And it will do this if it uses the superimage's stride to jump from the start of subimage data for one line to the start of subimage data for the next line.

I've filed https://gitlab.freedesktop.org/cairo/cairo/-/merge_requests/337

Whenever we're ready to land that patch, let's do so in a new bug please since this was was resolved nearly 5 months ago.

I'd be on board with landing it in bug 1769429, fwiw.

I fixed this in bug 1778158