Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Status

()

Core
Graphics
RESOLVED FIXED
15 years ago
6 years ago

People

(Reporter: blizzard, Assigned: zwol)

Tracking

(Blocks: 1 bug)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(8 attachments)

(Reporter)

Description

15 years ago
After the checkin for bug 126919 none of these apis are used anywhere.  See that
bug for more details.
Assignee: jag → nobody
(Assignee)

Comment 1

6 years ago
nsIWidget::SetFont seems to be gone, and I have a patch series deCOMming nsIFontMetrics that nukes GetFontHandle as a side effect, so I'm stealing this bug to carry that patch series.  Hope you don't mind.
Blocks: 105431
Status: NEW → ASSIGNED
Component: XUL → Graphics
Depends on: 266236
OS: Linux → All
QA Contact: jrgmorrison → thebes
Hardware: x86 → All
Summary: remove nsIFontMetrics::GetFontHandle and nsIWidget::SetFont() → deCOM nsIFontMetrics
(Assignee)

Comment 2

6 years ago
Created attachment 525578 [details] [diff] [review]
part 1: include minimization

Mostly-mechanical patch to remove all unnecessary #includes of ns*FontMetrics.h.
Attachment #525578 - Flags: review?(roc)
(Assignee)

Comment 3

6 years ago
Created attachment 525579 [details] [diff] [review]
part 2: merge nsIThebesFontMetrics into nsIFontMetrics

What it says.  Note I did not bump the nsIFontMetrics IID, because the very next patch makes the IID go away.
Attachment #525579 - Flags: review?(roc)
(Assignee)

Comment 4

6 years ago
Created attachment 525580 [details] [diff] [review]
part 3: deCOM ns(I|Thebes)FontMetrics.  Code outside gfx/ not fixed up.

This is the main deCOM patch.  gfx/ can be compiled at this point, but the mechanical fixups needed for the rest of the tree are in the next patch.
Attachment #525580 - Flags: review?(roc)
(Assignee)

Comment 5

6 years ago
Created attachment 525581 [details] [diff] [review]
part 4: mechanical fixups

perl -pi -e 's/\bns(?:I|Thebes)(?=FontMetrics\b)/ns/g;
             s/\bnsCOMPtr(?=<nsFontMetrics>)/nsRefPtr/g' [every file in the tree]
(Assignee)

Updated

6 years ago
Attachment #525581 - Flags: review?(roc)
(Assignee)

Comment 6

6 years ago
Created attachment 525582 [details] [diff] [review]
part 5: prune unused and unimplemented methods.

One interesting thing about this patch is that I deleted GetHeight, which was used but documented as "will be removed once callers have been fixed".  It did the same thing as GetMaxHeight, so I changed all remaining callers to use that.  Also there are a couple of technically-unrelated removals of unused variables in here (I was tired of the warnings).
Attachment #525582 - Flags: review?(roc)
(Assignee)

Comment 7

6 years ago
Created attachment 525583 [details] [diff] [review]
part 6: eliminate pointless nsresult return values.

Also rearranges code in nsFontMetrics.cpp a little so we can make use of the anonymous namespace for private classes.  The accessors that now return one nscoord also have the "Get" removed from their names.
Attachment #525583 - Flags: review?(roc)
(Assignee)

Comment 8

6 years ago
Created attachment 525584 [details] [diff] [review]
part 7: remove unnecessary members from nsFontMetrics

All other places that call CreateFontGroup put the gfxFontStyle argument on the stack, and nothing else in nsFontMetrics uses the gfxFontStyle, so we don't need to heap-allocate that and keep it around for the lifetime of the font metrics.  mFontGroup is a refptr so it doesn't need to be nulled in the constructor.  And there were *two* booleans for whether text runs were right-to-left, one of which was never set to true (but that was the one that was actually getting *used*; I assumed this was just a mistake).

This is as far as this patch series goes.  I have work past this point, trying to disentangle nsFontMetrics from nsThebesDeviceContext, but it's still got bugs in it, and this is enough for one bug.  This set + the set in bug 266236 has passed the try server.
Attachment #525584 - Flags: review?(roc)
(Assignee)

Updated

6 years ago
Assignee: nobody → zackw
(Assignee)

Updated

6 years ago
Depends on: 648197
Attachment #525578 - Flags: review?(roc) → review+
Attachment #525579 - Flags: review?(roc) → review+
Attachment #525580 - Flags: review?(roc) → review+
Attachment #525581 - Flags: review?(roc) → review+
Attachment #525582 - Flags: review?(roc) → review+
GetLanguage could return a raw nsIAtom*.
Attachment #525583 - Flags: review?(roc) → review+
Attachment #525584 - Flags: review?(roc) → review+
(Assignee)

Comment 10

6 years ago
(In reply to comment #9)
> GetLanguage could return a raw nsIAtom*.

Good point; I'll tack on another patch to do that tomorrow morning.
(Assignee)

Comment 11

6 years ago
Created attachment 526019 [details] [diff] [review]
part 8: GetLanguage() doesn't need to addref

Now I remember why I didn't do this in the first place: the only user is nsFontCache::GetMetricsFor, and I was originally going to have part 8 of this bug delete nsFontCache, so it seemed like unnecessary churn.  But that change turns out to expose a bunch of subtle bugs, so this is a better stopping place in case we wind up not being able to delete nsFontCache.

This series is waiting for one more review in bug 648197 before it can land, by the way.
Attachment #526019 - Flags: review?(roc)
Attachment #526019 - Flags: review?(roc) → review+
(Assignee)

Comment 12

6 years ago
http://hg.mozilla.org/mozilla-central/rev/34e0e1992231
http://hg.mozilla.org/mozilla-central/rev/84c4a1cc3655
http://hg.mozilla.org/mozilla-central/rev/2397bc9933ea
http://hg.mozilla.org/mozilla-central/rev/d06f1282cc6d
http://hg.mozilla.org/mozilla-central/rev/b339ef823c29
http://hg.mozilla.org/mozilla-central/rev/90a2c72237e2
http://hg.mozilla.org/mozilla-central/rev/ea77c7167870
http://hg.mozilla.org/mozilla-central/rev/ee377a1a5e31
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.