fix(sync): resolve CRE positions structurally with text as verification
Drop-cap markup like <p><span>C</span>onvergence of Heaven and Earth</p> made getTextFromXPointer return just 'C'. The text search then matched the first 'C' in the chapter and stored doc-start (/4/2/1:0) with 'precision: exact', so the web reader reopened at the chapter start while the percentage looked mid-chapter. - Add convertByStructuralPath: walk the parsed CRE ElementPath against the raw XHTML (same-tag 1-based indexing, mirroring buildCREXPointer), map CharOffset into the target element's text, and build the CFI. Usable device text verifies the landing; disagreement falls through instead of storing a confident-but-wrong CFI. - Add usableContextText guard (>=8 runes, >=2 words): single chars can never claim an exact text-search hit in either direction (ConvertCREToStandard and reverseByTextSearch). - Add drop-cap regression fixtures plus usable-context unit tests. - Verified against the real book: DocFragment[26]/p[12]/span header now converts to epubcfi(/6/52!/4/28/2/1:0) structural both with 'C' and the full header, and round-trips back to DocFragment[26].
This commit is contained in:
@@ -324,16 +324,233 @@ func (c *CFIConverter) ConvertCREToStandard(xpointer string, storedPercentage fl
|
|||||||
}, nil
|
}, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
if contextText != "" {
|
// Primary: structural walk of the CRE element path. This is exact when
|
||||||
result := c.convertByTextSearch(body, xp, spine, href, storedPercentage, contextText)
|
// the live CRE DOM still lines up with the raw XHTML (same-tag indices,
|
||||||
if result != nil {
|
// see buildCREXPointer for the counting rule). Text is the final
|
||||||
|
// verification, not a fallback: a usable context that disagrees with
|
||||||
|
// the structural landing rejects it so text search gets a chance.
|
||||||
|
if len(xp.ElementPath) > 0 {
|
||||||
|
if result := c.convertByStructuralPath(body, xp, spine, href, storedPercentage, contextText); result != nil {
|
||||||
return result, nil
|
return result, nil
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if usable, normalized := usableContextText(contextText); usable {
|
||||||
|
result := c.convertByTextSearch(body, xp, spine, href, storedPercentage, normalized)
|
||||||
|
if result != nil {
|
||||||
|
return result, nil
|
||||||
|
}
|
||||||
|
} else if normalizeWhitespace(contextText) != "" {
|
||||||
|
log.Printf("Bookhoard: ignoring too-short context %q for text search in %s (structural missed)", contextText, href)
|
||||||
|
}
|
||||||
|
|
||||||
return c.convertByPercentageOffset(body, xp, spine, href, storedPercentage)
|
return c.convertByPercentageOffset(body, xp, spine, href, storedPercentage)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// usableContextText reports whether a device context string is distinctive
|
||||||
|
// enough to claim an "exact" text-search hit. Single characters from split
|
||||||
|
// inline markup (e.g. drop-cap <span>C</span>onvergence) must never qualify:
|
||||||
|
// findTextInNode would match the first "C" in the chapter (doc start).
|
||||||
|
// Callers treat unusable as empty and fall through to the next strategy.
|
||||||
|
func usableContextText(s string) (bool, string) {
|
||||||
|
normalized := normalizeWhitespace(s)
|
||||||
|
if normalized == "" {
|
||||||
|
return false, ""
|
||||||
|
}
|
||||||
|
if utf8.RuneCountInString(normalized) < 8 {
|
||||||
|
return false, ""
|
||||||
|
}
|
||||||
|
if len(strings.Fields(normalized)) < 2 {
|
||||||
|
return false, ""
|
||||||
|
}
|
||||||
|
return true, normalized
|
||||||
|
}
|
||||||
|
|
||||||
|
// resolveCREElementPath walks a parsed CRE element path against the raw
|
||||||
|
// XHTML body. Indexing mirrors buildCREXPointer: 1-based nth element with
|
||||||
|
// the same tag among element siblings (case-insensitive). Returns nil when
|
||||||
|
// the live CRE DOM has drifted too far from disk (splits, injected wrappers)
|
||||||
|
// so callers can fall through instead of landing on the wrong node.
|
||||||
|
func resolveCREElementPath(body *html.Node, path []pathStep) *html.Node {
|
||||||
|
current := body
|
||||||
|
for _, step := range path {
|
||||||
|
if step.tag == "" {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
if strings.EqualFold(step.tag, "body") {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
var matches []*html.Node
|
||||||
|
for child := current.FirstChild; child != nil; child = child.NextSibling {
|
||||||
|
if child.Type == html.ElementNode && strings.EqualFold(child.Data, step.tag) {
|
||||||
|
matches = append(matches, child)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if len(matches) == 0 {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
idx := step.index - 1
|
||||||
|
if idx < 0 || idx >= len(matches) {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
current = matches[idx]
|
||||||
|
}
|
||||||
|
if current == body {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
return current
|
||||||
|
}
|
||||||
|
|
||||||
|
// firstTextDescendant returns the first text node with non-whitespace content
|
||||||
|
// under n (document order), for structural landings on elements.
|
||||||
|
func firstTextDescendant(n *html.Node) *html.Node {
|
||||||
|
var found *html.Node
|
||||||
|
var walk func(*html.Node) bool
|
||||||
|
walk = func(node *html.Node) bool {
|
||||||
|
if node.Type == html.TextNode && strings.TrimSpace(node.Data) != "" {
|
||||||
|
found = node
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
for child := node.FirstChild; child != nil; child = child.NextSibling {
|
||||||
|
if walk(child) {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
walk(n)
|
||||||
|
return found
|
||||||
|
}
|
||||||
|
|
||||||
|
// textNodeAtRuneOffset walks text nodes under elem in document order and
|
||||||
|
// returns the node containing the rune offset plus the local offset within
|
||||||
|
// that node. Offsets beyond the end clamp to the last node.
|
||||||
|
func textNodeAtRuneOffset(elem *html.Node, offset int) (*html.Node, int) {
|
||||||
|
if offset < 0 {
|
||||||
|
offset = 0
|
||||||
|
}
|
||||||
|
var target *html.Node
|
||||||
|
local := 0
|
||||||
|
remaining := offset
|
||||||
|
var walk func(*html.Node) bool
|
||||||
|
walk = func(node *html.Node) bool {
|
||||||
|
if node.Type == html.TextNode {
|
||||||
|
length := utf8.RuneCountInString(node.Data)
|
||||||
|
if remaining < length {
|
||||||
|
target = node
|
||||||
|
local = remaining
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
remaining -= length
|
||||||
|
target = node
|
||||||
|
local = length
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
for child := node.FirstChild; child != nil; child = child.NextSibling {
|
||||||
|
if walk(child) {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
walk(elem)
|
||||||
|
return target, local
|
||||||
|
}
|
||||||
|
|
||||||
|
// blockFlattenedText returns the flattened visible text of the nearest
|
||||||
|
// non-inline block ancestor (span/a/em/etc. are transparent), so drop-cap
|
||||||
|
// splits like <span>C</span>onvergence verify against the full words.
|
||||||
|
func blockFlattenedText(textNode *html.Node) string {
|
||||||
|
block := findBlockParent(textNode)
|
||||||
|
if block == nil {
|
||||||
|
if textNode.Parent != nil {
|
||||||
|
block = textNode.Parent
|
||||||
|
} else {
|
||||||
|
return strings.TrimSpace(textNode.Data)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
segments := collectInlineText(block)
|
||||||
|
var runes []rune
|
||||||
|
for _, seg := range segments {
|
||||||
|
runes = append(runes, seg.runes...)
|
||||||
|
}
|
||||||
|
// Include leading direct text children the inline walk skips.
|
||||||
|
var direct []rune
|
||||||
|
for child := block.FirstChild; child != nil; child = child.NextSibling {
|
||||||
|
if child.Type == html.TextNode {
|
||||||
|
direct = append(direct, []rune(child.Data)...)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
combined := string(direct)
|
||||||
|
if flattened := string(runes); flattened != "" {
|
||||||
|
if combined != "" {
|
||||||
|
combined += " " + flattened
|
||||||
|
} else {
|
||||||
|
combined = flattened
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return normalizeWhitespace(combined)
|
||||||
|
}
|
||||||
|
|
||||||
|
func (c *CFIConverter) convertByStructuralPath(body *html.Node, xp *CREXPointer, spine *spineCache, href string, storedPercentage float64, contextText string) *ConversionResult {
|
||||||
|
elem := resolveCREElementPath(body, xp.ElementPath)
|
||||||
|
if elem == nil {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
var textNode *html.Node
|
||||||
|
var localOffset int
|
||||||
|
if xp.CharOffset > 0 {
|
||||||
|
textNode, localOffset = textNodeAtRuneOffset(elem, xp.CharOffset)
|
||||||
|
} else {
|
||||||
|
textNode = firstTextDescendant(elem)
|
||||||
|
localOffset = 0
|
||||||
|
}
|
||||||
|
if textNode == nil {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// Text is the final verification: when the device sent usable words and
|
||||||
|
// they disagree with this structural landing, reject it and let text
|
||||||
|
// search / percentage decide rather than storing a confident-but-wrong CFI.
|
||||||
|
if usable, normalized := usableContextText(contextText); usable {
|
||||||
|
flat := blockFlattenedText(textNode)
|
||||||
|
if flat != "" && !strings.Contains(flat, normalized) && !strings.Contains(normalized, flat) {
|
||||||
|
// Compare a prefix too: device sends ~100 chars from the reader
|
||||||
|
// position while the block may be longer.
|
||||||
|
prefix := normalized
|
||||||
|
if utf8.RuneCountInString(prefix) > 40 {
|
||||||
|
runes := []rune(prefix)
|
||||||
|
prefix = string(runes[:40])
|
||||||
|
}
|
||||||
|
if !strings.Contains(flat, prefix) {
|
||||||
|
log.Printf("Bookhoard: structural landing disagrees with context in %s (block %q vs ctx %q)", href, truncateForLog(flat, 80), truncateForLog(normalized, 80))
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
spineIndex := xp.FragmentIndex - 1
|
||||||
|
cfi, err := buildCFI(spineIndex, textNode, localOffset)
|
||||||
|
if err != nil || cfi == "" {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
log.Printf("Bookhoard: structural match → %s (precision: structural)", cfi)
|
||||||
|
return &ConversionResult{
|
||||||
|
EPUBCFI: cfi,
|
||||||
|
Href: href,
|
||||||
|
Percentage: storedPercentage,
|
||||||
|
Precision: "structural",
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func truncateForLog(s string, n int) string {
|
||||||
|
if utf8.RuneCountInString(s) <= n {
|
||||||
|
return s
|
||||||
|
}
|
||||||
|
return string([]rune(s)[:n]) + "…"
|
||||||
|
}
|
||||||
|
|
||||||
func (c *CFIConverter) convertFragmentID(s string, storedPercentage float64) (*ConversionResult, error) {
|
func (c *CFIConverter) convertFragmentID(s string, storedPercentage float64) (*ConversionResult, error) {
|
||||||
frag, err := ParseCREFragmentID(s)
|
frag, err := ParseCREFragmentID(s)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -1306,8 +1523,11 @@ func (c *CFIConverter) ConvertStandardToCRE(epubcfi string, storedPercentage flo
|
|||||||
}
|
}
|
||||||
|
|
||||||
func (c *CFIConverter) reverseByTextSearch(epubcfi string, storedPercentage float64, contextText string) (*ReverseConversionResult, error) {
|
func (c *CFIConverter) reverseByTextSearch(epubcfi string, storedPercentage float64, contextText string) (*ReverseConversionResult, error) {
|
||||||
normalizedCtx := normalizeWhitespace(contextText)
|
usable, normalizedCtx := usableContextText(contextText)
|
||||||
if normalizedCtx == "" {
|
if !usable {
|
||||||
|
if normalizeWhitespace(contextText) != "" {
|
||||||
|
log.Printf("Bookhoard: CFI→CRE ignoring too-short context %q", contextText)
|
||||||
|
}
|
||||||
return &ReverseConversionResult{
|
return &ReverseConversionResult{
|
||||||
Precision: "percentage",
|
Precision: "percentage",
|
||||||
Percentage: storedPercentage,
|
Percentage: storedPercentage,
|
||||||
|
|||||||
@@ -519,3 +519,150 @@ func parseTestHTML(s string) *html.Node {
|
|||||||
}
|
}
|
||||||
return doc
|
return doc
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestUsableContextText(t *testing.T) {
|
||||||
|
if ok, _ := usableContextText("C"); ok {
|
||||||
|
t.Error("single char must not be usable context")
|
||||||
|
}
|
||||||
|
if ok, _ := usableContextText("Chapter"); ok {
|
||||||
|
t.Error("single word must not be usable context")
|
||||||
|
}
|
||||||
|
if ok, norm := usableContextText("Convergence of Heaven and Earth"); !ok || norm == "" {
|
||||||
|
t.Error("full header must be usable context")
|
||||||
|
}
|
||||||
|
if ok, _ := usableContextText(""); ok {
|
||||||
|
t.Error("empty must not be usable context")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Drop-cap regression: <p><span>C</span>onvergence of Heaven and Earth</p>.
|
||||||
|
// A device sitting on that header used to send context "C", which text
|
||||||
|
// search matched at the chapter heading ("Chapter 10") and stored doc-start
|
||||||
|
// (/4/2/1:0). Structural resolution must land in the header paragraph.
|
||||||
|
func writeDropCapEPUB(t *testing.T) string {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
docs := []struct {
|
||||||
|
name string
|
||||||
|
body string
|
||||||
|
}{
|
||||||
|
{"ch9.xhtml", `<body><h1>Chapter 9</h1><h1>Is the Kaphar of Christ the Gospel?</h1><p>Opening of chapter nine with some text.</p></body>`},
|
||||||
|
{"ch10.xhtml", `<body><h1>Chapter 10</h1><h1>The Three C's of the New Covenant</h1><p>The <span>C</span>leansing Life of Christ</p><p>Present yourselves as slaves for obedience, you are slaves of that same one whom you obey, either of sin resulting in death.</p><p><span>C</span>onvergence of Heaven and Earth</p><p>The overarching goal of Christ is to converge heaven and earth.</p></body>`},
|
||||||
|
}
|
||||||
|
|
||||||
|
containerXML := `<?xml version="1.0"?>
|
||||||
|
<container version="1.0" xmlns="urn:oasis:names:tc:opendocument:xmlns:container">
|
||||||
|
<rootfiles>
|
||||||
|
<rootfile full-path="OEBPS/content.opf" media-type="application/oebps-package+xml"/>
|
||||||
|
</rootfiles>
|
||||||
|
</container>`
|
||||||
|
|
||||||
|
manifest := ""
|
||||||
|
spineRefs := ""
|
||||||
|
for _, d := range docs {
|
||||||
|
id := d.name[:len(d.name)-len(".xhtml")]
|
||||||
|
manifest += " <item id=\"" + id + "\" href=\"" + d.name + "\" media-type=\"application/xhtml+xml\"/>\n"
|
||||||
|
spineRefs += " <itemref idref=\"" + id + "\"/>\n"
|
||||||
|
}
|
||||||
|
opf := `<?xml version="1.0" encoding="UTF-8"?>
|
||||||
|
<package xmlns="http://www.idpf.org/2007/opf" version="3.0" unique-identifier="uid">
|
||||||
|
<metadata xmlns:dc="http://purl.org/dc/elements/1.1/">
|
||||||
|
<dc:identifier id="uid">dropcap-fixture</dc:identifier>
|
||||||
|
<dc:title>Dropcap Fixture</dc:title>
|
||||||
|
</metadata>
|
||||||
|
<manifest>
|
||||||
|
` + manifest + ` </manifest>
|
||||||
|
<spine>
|
||||||
|
` + spineRefs + ` </spine>
|
||||||
|
</package>`
|
||||||
|
|
||||||
|
path := t.TempDir() + "/dropcap.epub"
|
||||||
|
f, err := os.Create(path)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
defer f.Close()
|
||||||
|
zw := zip.NewWriter(f)
|
||||||
|
write := func(name, content string) {
|
||||||
|
w, err := zw.Create(name)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if _, err := w.Write([]byte(content)); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
write("META-INF/container.xml", containerXML)
|
||||||
|
write("OEBPS/content.opf", opf)
|
||||||
|
for _, d := range docs {
|
||||||
|
write("OEBPS/"+d.name, "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<html xmlns=\"http://www.w3.org/1999/xhtml\">"+d.body+"</html>\n")
|
||||||
|
}
|
||||||
|
if err := zw.Close(); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
return path
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestDropCapSingleCharDoesNotHitDocStart(t *testing.T) {
|
||||||
|
c := NewCFIConverter(writeDropCapEPUB(t))
|
||||||
|
|
||||||
|
// CRE xpointer into the drop-cap header paragraph:
|
||||||
|
// DocFragment[2] = ch10 (1-based), body/p[3] = Convergence header
|
||||||
|
// (same-tag indexing, mirrors buildCREXPointer).
|
||||||
|
xp := "/body/DocFragment[2]/body/p[3]/span[1]/text().0"
|
||||||
|
result, err := c.ConvertCREToStandard(xp, 0.52, "C")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("ConvertCREToStandard error: %v", err)
|
||||||
|
}
|
||||||
|
t.Logf("Drop-cap single-char → %s (%s)", result.EPUBCFI, result.Precision)
|
||||||
|
if result.EPUBCFI == "" {
|
||||||
|
t.Fatal("expected non-empty epubcfi")
|
||||||
|
}
|
||||||
|
if strings.HasSuffix(result.EPUBCFI, "/4/2/1:0)") {
|
||||||
|
t.Errorf("single-char context collapsed to doc start: %s", result.EPUBCFI)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestDropCapStructuralLandsInHeader(t *testing.T) {
|
||||||
|
c := NewCFIConverter(writeDropCapEPUB(t))
|
||||||
|
|
||||||
|
xp := "/body/DocFragment[2]/body/p[3]/span[1]/text().0"
|
||||||
|
result, err := c.ConvertCREToStandard(xp, 0.52, "Convergence of Heaven and Earth")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("ConvertCREToStandard error: %v", err)
|
||||||
|
}
|
||||||
|
t.Logf("Drop-cap structural → %s (%s)", result.EPUBCFI, result.Precision)
|
||||||
|
if result.EPUBCFI == "" {
|
||||||
|
t.Fatal("expected non-empty epubcfi")
|
||||||
|
}
|
||||||
|
if strings.HasSuffix(result.EPUBCFI, "/4/2/1:0)") {
|
||||||
|
t.Errorf("full header context still collapsed to doc start: %s", result.EPUBCFI)
|
||||||
|
}
|
||||||
|
if result.Precision != "structural" && result.Precision != "exact" {
|
||||||
|
t.Errorf("expected structural/exact precision, got %s", result.Precision)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Round-trip back to CRE must stay in the same fragment (web → mobile).
|
||||||
|
reverse, err := c.ConvertStandardToCRE(result.EPUBCFI, result.Percentage, "Convergence of Heaven and Earth")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("reverse conversion error: %v", err)
|
||||||
|
}
|
||||||
|
if reverse.XPointer == "" {
|
||||||
|
t.Fatal("expected non-empty reverse XPointer")
|
||||||
|
}
|
||||||
|
if !strings.Contains(reverse.XPointer, "DocFragment[2]") {
|
||||||
|
t.Errorf("expected reverse into DocFragment[2], got %s", reverse.XPointer)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestReverseIgnoresSingleCharContext(t *testing.T) {
|
||||||
|
c := NewCFIConverter(writeDropCapEPUB(t))
|
||||||
|
|
||||||
|
reverse, err := c.ConvertStandardToCRE("epubcfi(/6/4!/4/99999/1:0)", 0.5, "C")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("reverse conversion error: %v", err)
|
||||||
|
}
|
||||||
|
if reverse.Precision != "percentage" {
|
||||||
|
t.Errorf("single-char reverse context must fall back to percentage, got %s", reverse.Precision)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user