Created attachment 371982 [details]
MacOS Crash report for Edit Tax Information Dialog

This happens in MacOS, too. Crash report with stack trace attached.

The reason this fails is trying to get an account from an empty accounts Glist, one could test for this but I was trying to see why and came across this...

If you create a new xml file and try to configure this, it fails as it can not save the name and tax type.

In gnc-ui-util.c there are these two defines...

#define OPTION_TAXUS_NAME "book/tax_US/name"
#define OPTION_TAXUS_TYPE "book/tax_US/type"

and they are used in this function...

qof_book_set_string_option(gnc_get_current_book(), OPTION_TAXUS_NAME, tax_name);
qof_book_set_string_option(gnc_get_current_book(), OPTION_TAXUS_TYPE, tax_type);

in qofbook.c there is 

void
qof_book_set_string_option(QofBook* book, const char* opt_name, const char* opt_val)
{
    qof_book_begin_edit(book);
    auto frame = qof_instance_get_slots(QOF_INSTANCE(book));
    auto opt_path = opt_name_to_path(opt_name);
    if (opt_val && (*opt_val != '\0'))
        delete frame->set(opt_path, new KvpValue(g_strdup(opt_val)));
    else
        delete frame->set(opt_path, nullptr);
    qof_instance_set_dirty (QOF_INSTANCE (book));
    qof_book_commit_edit(book);
}

Do not understand why there is 'delete' unless it is supposed to delete an entry before creating a new one ?

The corresponding 'get' function works as I can copy this block from an old xml file and the entries are read...

  <slot>
    <slot:key>book</slot:key>
    <slot:value type="frame">
      <slot>
        <slot:key>tax_US</slot:key>
        <slot:value type="frame">
          <slot>
            <slot:key>name</slot:key>
            <slot:value type="string">Bob</slot:value>
          </slot>
          <slot>
            <slot:key>type</slot:key>
            <slot:value type="string">F1040</slot:value>
          </slot>
        </slot:value>
      </slot>
    </slot:value>
  </slot>

> Do not understand why there is 'delete' unless it is supposed to delete an 
> entry before creating a new one ?

It's deleting the old KvpValue* returned by KvpFrame::Set(const std::string&, KvpValue*). See https://github.com/Gnucash/gnucash/blob/maint/libgnucash/engine/kvp-frame.cpp#L103.

> The reason this fails is trying to get an account from an empty accounts 
> Glist, one could test for this but I was trying to see why

"Why" is because there's an income account selected and the filter is now set for expense, so nothing matches. It works the other way, too.

I tried clearing the selection but it gets set back somehow, and the GtkTreeViewSelection isn't transparent enough to set a watchpoint on it. I punted and added an "accounts == NULL" check, repeating the case 0 code if there aren't any accounts.

I had fixed that locally a different way by adding unselect all to gnc_tax_info_acct_type_cb with a belt and braces test for length of accounts greater than zero.

Its just when I tried with a new xml file, I came across the above and was going to fix both, the function qof_book_set_string_option does not seem to work with multiple Path elements for opt_path. If I change one of the defines to "booktax_USname" it works so it does not like multiple frames.

Bob, if you have a better fix by all means do a PR. 

For the KVP problem, opt_name_to_path should return a vector<string> with elements "book", "tax_us", and either "name" or "type". Is that what's failing?

John, I will pull maint and see if my changes might make it clearer to what is happening.

Yes, the KVP problem seems to be down to the number of elements, I think one and two are OK, but more than that it fails as it does not create the first lot of frames. This function is only used in three places as far as I can find, here, in a test file and in scm when you use File/properties/counters.

I can get it working by doing the following but you no doubt can make the c++ changes easier as I still find a lot of aspects confusing...

void
qof_book_set_string_option(QofBook* book, const char* opt_name, const char* opt_val)
{
    qof_book_begin_edit(book);
    auto frame = qof_instance_get_slots(QOF_INSTANCE(book));
    auto opt_path = opt_name_to_path(opt_name);

    if (int(opt_path.size() == 3))
    {
        auto first_slot = frame->get_slot({opt_path[0]});
        if (!first_slot)
            frame->set({opt_path[0]}, new KvpValue{new KvpFrame});

        first_slot = frame->get_slot({opt_path[0]});
        auto first_frame = first_slot->get<KvpFrame*>();

        auto next_slot = first_frame->get_slot({opt_path[1]});
        if (!next_slot)
            first_frame->set({opt_path[1]}, new KvpValue{new KvpFrame});

        if (opt_val && (*opt_val != '\0'))
            delete first_frame->set({opt_path[1], opt_path[2]}, new KvpValue(g_strdup(opt_val)));
        else
            delete first_frame->set({opt_path[1], opt_path[2]}, nullptr);
    }
    else
    {
        if (opt_val && (*opt_val != '\0'))
            delete frame->set(opt_path, new KvpValue(g_strdup(opt_val)));
        else
            delete frame->set(opt_path, nullptr);
    }
    qof_instance_set_dirty (QOF_INSTANCE (book));
    qof_book_commit_edit(book);
}

GnuCash bug tracking has moved to a new Bugzilla host. This bug has been copied to https://bugs.gnucash.org/show_bug.cgi?id=796079. Please update any external references or bookmarks.