Is this a commit?

If so - is it normal to do indention changes and actual code changes in the same commit?

Personally, I would first have committed the indention changes and then did a second commit with the coded changes.

If you look closely, you'll see that it's not merely an indentation change. The bulk of the function used to be inside a large condition, but that has been changed to an early return. Still, it would have been a little nicer if it had been done in two commits.

Here's a better view of it: https://phabricator.services.mozilla.com/rMOZILLACENTRAL8ae3...

Basically it just adds a one line check near the top of a ShowTooltip() function for whether "doc->HasFocus(IgnoreErrors())", and, if not, returns early.

The indentation changes are because of a removed if block.

i always struggle with this. i usually end up with code changes first because i want to test code before committing, which means i can't commit a whitespace change before i know the code change works.

and every time i think about the problem i stumble over python where the two can't be separated.

i believe in the end a better solution would be to mark whitespace changes in a different color. or even better mark each character that changed, not just the line.

in other words: we want better diff tools

Gerrit is capable of showing only non-whitespace diffs.

Wow, if you exclude lines changed in the commit due to indenting changing, there are only five new lines of code for this change!

The changes, in sum, in nsXULTooltipListener.cpp

    -  if (tooltipNode->GetComposedDoc() &&
    -      nsContentUtils::IsChromeDoc(tooltipNode->GetComposedDoc())) {
    +   // Make sure the document still has focus.
    +   auto* doc = tooltipNode->GetComposedDoc();
    +   if (!doc || !nsContentUtils::IsChromeDoc(doc) ||
    +       !doc->HasFocus(IgnoreErrors())) {
    +     return NS_OK;
    +   }
    ...
    -   }
    }
    return NS_OK;

If I see correctly, all the changes are:

1) remembering result of tooltipNode->GetComposedDoc() and adding the test of doc->HasFocus(IgnoreErrors()). Note that writing this one now is maybe easier than it was at the time the initial code was written, it could be the "auto" in this current semantic didn't exist in C++ (or the used compilers/platforms) at that time.

2) Explicit return. Instead of:

    if (b)
      X;
    return OK;

now it's:

    if (!b)
      return OK;
    X;
    return OK; 

which in this case increases readability as X is in many lines and b is a more complex condition.

Can any one give a brief synopsis of the state of XUL and gecko in Firefox/Mozilla? Are the XUL (XULRunner) and gecko runtimes still actively worked on? Or has it been absorbed into the Firefox runtime long ago?

I recall reading some time ago that the work on XULRunner essentially came to a halt, and that components in Firefox/Mozilla that depend on XUL would slowly be phased out.

But does that mean that work on the XUL runtime and components in Firefox also effectively came to a halt? I figured that most of these really old XUL bugs in Firefox were never going to be fixed, instead replacing a whole layer/component dependent on XUL was seen as a better use of time and resources.

Edit: anyone have a good diagram of the layers/components in Firefox. Something that can illustrate where Quantum, Gecko, XUL, etc all live in Firefox. It would be really cool (doubt it exists) if there was an animated diagram that would show the changes of this stack overtime.

Most of the challenges of a bug isn't the fix, but rather figuring out the behaviour.

Especially true if breakpoints don't work :)

Always a bummer when your tools for debugging don’t work. :/