It feels safer to use a function with a new name, rather than just changing the
behaviour of the existing function.
For most of these cases the PL_DHashTableLookup() result was checked with
PL_DHASH_ENTRY_IS_{FREE,BUSY} so the conversion was easy. A few of them
preceded that check with a useless null check, but the intent of these was
still easy to determine.
I'll do the trickier ones in subsequent patches.
--HG--
extra : rebase_source : ab37a7a30be563861ded8631771181aacf054fd4
We do not want to traverse inside native anonymous elements, but we
should still be able to skip over generated content, to avoid getting
stuck on such images.
The caret movement code already handles unselectable text frames if we
happen to land in the middle of one in nsTextFrame::PeekOffsetCharacter/Word.
However, when performing frame traversal to find the next frame to jump
to, we don't remember if we skipped over an unselectable frame, which causes
us to jump one offset too much when the caret is on the boundary of
selectable and unselectable content. The test cases demonstrate the
scenario. Note that an <img alt=foo> is implemented by adding a
generated content to the inline frame representing it, so as far as
the caret movement code is concerned, both test cases are treated similarly.
Note that we need to do this only when moving the selection, and not
when extending it. We are adding an aExtend argument to
nsPeekOffsetStruct's constructor in order to be able to special case
that.
We do not want to traverse inside native anonymous elements, but we
should still be able to skip over generated content, to avoid getting
stuck on such images.
The caret movement code already handles unselectable text frames if we
happen to land in the middle of one in nsTextFrame::PeekOffsetCharacter/Word.
However, when performing frame traversal to find the next frame to jump
to, we don't remember if we skipped over an unselectable frame, which causes
us to jump one offset too much when the caret is on the boundary of
selectable and unselectable content. The test cases demonstrate the
scenario. Note that an <img alt=foo> is implemented by adding a
generated content to the inline frame representing it, so as far as
the caret movement code is concerned, both test cases are treated similarly.
Note that we need to do this only when moving the selection, and not
when extending it. We are adding an aExtend argument to
nsPeekOffsetStruct's constructor in order to be able to special case
that.
Sometimes, in very specific cases, the visible region gets simplified to one rect and is thus much bigger than the draw region. This becomes a problem if we decide to pull an opaque background color from a lower layer so that we are opaque. In which case we draw the background color over the whole visible region. But we use the draw region to determine if we can place items below this layer, so that background color could cover them incorrectly.
The content inside an editable region is either editable itself, or
is inside a contenteditable="false" subtree. In the first case,
it should not be focusable since it is editable. In the second
case, it should not be focusable since the entire non-editable
region is treated as a special single entity for the purposes of
selection and caret movement, and having something focusable in
the middle of such a subtree breaks that model.
The content inside an editable region is either editable itself, or
is inside a contenteditable="false" subtree. In the first case,
it should not be focusable since it is editable. In the second
case, it should not be focusable since the entire non-editable
region is treated as a special single entity for the purposes of
selection and caret movement, and having something focusable in
the middle of such a subtree breaks that model.
Currently the setting of PLDHashTable::ops is very haphazard.
- PLDHashTable has no constructor, so it's not auto-nulled, so lots of places
null it themselves.
- In the fallible PLDHashTable::Init() function, if the entry storage
allocation fails we'll be left with a table that has |ops| set -- indicating
it's been initialized -- but has null entry storage. I'm not certain this can
cause problems but it feels unsafe, and some (but not all) callers of Init()
null it on failure.
- PLDHashTable does not null |ops| in Finish(), so some (but not all) callers
do this themselves.
This patch makes things simpler.
- It adds a constructor that zeroes |ops|.
- It modifies Init() so that it only sets |ops| once success is ensured.
- It zeroes |ops| in Finish().
- Finally, it removes all the now-unnecessary |ops| nulling done by the users
of PLDHashTable.
--HG--
extra : rebase_source : bb34979c218d152562a2f9c7e5215256c111cc5b
I noticed the GetRootPresContext call being expensive in a profile that
involved painting in a (non-e10s) window with around 400-500 tabs.
Moving the mIsActive test (most likely to be false) first should fix
that.
