Re: white-space patch
Johannes Hofmann <[email protected]>
| Newsgroups | gmane.comp.web.dillo.devel |
|---|---|
| Message-ID | <[email protected]> |
On Wed, Oct 05, 2011 at 05:54:22PM +0200, Johannes Hofmann wrote:
> On Wed, Oct 05, 2011 at 12:48:56PM -0300, Jorge Arellano Cid wrote:
> > On Wed, Oct 05, 2011 at 01:32:29PM +0200, Johannes Hofmann wrote:
> > > On Tue, Oct 04, 2011 at 04:13:57PM -0300, Jorge Arellano Cid wrote:
> > > > On Tue, Oct 04, 2011 at 01:57:05PM -0300, Jorge Arellano Cid wrote:
> > > >
> > > > > [...]
> > > > > Attached goes a small and very interesting testcase.
> > > > > (I had a hard time reducing google's page :-P).
> > > > >
> > > > > Compare with Firefox/Iceweasel rendering.
> > > >
> > > > There's a "min-width" CSS element that dillo doesn't support:
> > > >
> > > > #center_col{min-width:562px}
> > > >
> > > > Taking it out of the equation makes Firefox render the page
> > > > quite similar to dillo; so maybe it is not a rewrap bug, but a
> > > > lack of implementation of CSS' min-width.
> > >
> > > A colleague just found a simple test case, and the fix for it also
> > > seems to fix the google case - but it needs some more testing.
> > >
> > > Testcase:
> > >
> > > <b>Hello World</b><div>Hello world</div>
> > >
> > > Notice how the first line breaks without need when narrowing the
> > > window.
> >
> > Good testcase!
> >
> > Here, the first sentence is broken regardless the window size,
> > and FWIW, this patch solves it:
> >
> > @@ -1825,6 +1825,7 @@ void Textblock::addParbreak (int space,
> >
> > word = addWord (0, 0, 0, style);
> > word->content.type = core::Content::BREAK;
> > + word->content.breakType = core::Content::BREAK_OK;
> > word->content.breakSpace = space;
> > wordWrap (words->size () - 1);
> > }
> > @@ -1846,6 +1847,7 @@ void Textblock::addLinebreak (core::styl
> > word = addWord (0, 0, 0, style);
> >
> > word->content.type = core::Content::BREAK;
> > + word->content.breakType = core::Content::BREAK_OK;
> > word->content.breakSpace = 0;
> > wordWrap (words->size () - 1);
> > }
> >
> >
> > (this patch doesn't solve the google testcase, so I thought
> > it was better to let you know of it).
>
> Thanks! I already have a fix and I think the whole thing can
> be committed tonight or tomorrow.
Ok, another round of testing. Please check how attached patch works
for you.
Cheers,
Johannes
PS: Kudos to Florian who by accident found the nice test case!
_______________________________________________
Dillo-dev mailing list
[email protected]
http://lists.auriga.wearlab.de/cgi-bin/mailman/listinfo/dillo-dev
whitespace-fixes3.diff
(text/plain, 15.2 KB)
diff --git a/dw/textblock.cc b/dw/textblock.cc
--- a/dw/textblock.cc
+++ b/dw/textblock.cc
@@ -73,7 +73,6 @@
//DBG_OBJ_SET_NUM(page, "num_lines", num_lines);
lastLineWidth = 0;
- lastLineParMin = 0;
lastLineParMax = 0;
wrapRef = -1;
@@ -202,10 +201,9 @@
{
core::Extremes wordExtremes;
Line *line;
- Word *word;
+ Word *word, *prevWord = NULL;
int wordIndex, lineIndex;
- int parMin, parMax;
- bool nowrap;
+ int parMax;
//DBG_MSG (widget, "extremes", 0, "Dw_page_get_extremes");
//DBG_MSG_START (widget);
@@ -221,7 +219,7 @@
* words[first_word]->nowrap set is no longer necessary, since
* Dw_page_real_word_wrap sets max_word_min to the correct value in any
* case. */
- extremes->minWidth = line->maxWordMin;
+ extremes->minWidth = line->maxParMin;
extremes->maxWidth = misc::max (line->maxParMax, lastLineParMax);
//DBG_MSG (widget, "extremes", 0, "simple case");
} else {
@@ -232,13 +230,11 @@
if (wrapRef == 0) {
extremes->minWidth = 0;
extremes->maxWidth = 0;
- parMin = 0;
parMax = 0;
} else {
line = lines->getRef (wrapRef);
- extremes->minWidth = line->maxWordMin;
+ extremes->minWidth = line->maxParMin;
extremes->maxWidth = line->maxParMax;
- parMin = line->parMin;
parMax = line->parMax;
//DBG_MSGF (widget, "extremes", 0, "parMin = %d", parMin);
@@ -250,15 +246,9 @@
for (lineIndex = wrapRef; lineIndex < lines->size (); lineIndex++) {
//DBG_MSGF (widget, "extremes", 0, "line %d", lineIndex);
//DBG_MSG_START (widget);
- core::style::WhiteSpace ws;
+ int parMin = 0;
line = lines->getRef (lineIndex);
- ws = words->getRef(line->firstWord)->style->whiteSpace;
- nowrap = ws == core::style::WHITE_SPACE_PRE ||
- ws == core::style::WHITE_SPACE_NOWRAP;
-
- //DEBUG_MSG (DEBUG_SIZE_LEVEL, " line %d (of %d), nowrap = %d\n",
- // lineIndex, page->num_lines, nowrap);
for (wordIndex = line->firstWord; wordIndex <= line->lastWord;
wordIndex++) {
@@ -272,20 +262,28 @@
// " (next plus %d)\n", page->line1_offset);
}
- if (nowrap) {
- parMin += prevWordSpace + wordExtremes.minWidth;
- //DBG_MSGF (widget, "extremes", 0, "parMin = %d", parMin);
- } else {
- if (extremes->minWidth < wordExtremes.minWidth)
- extremes->minWidth = wordExtremes.minWidth;
- }
+
+ if (extremes->minWidth < wordExtremes.minWidth)
+ extremes->minWidth = wordExtremes.minWidth;
_MSG("parMax = %d, wordMaxWidth=%d, prevWordSpace=%d\n",
parMax, wordExtremes.maxWidth, prevWordSpace);
if (word->content.type != core::Content::BREAK)
parMax += prevWordSpace;
parMax += wordExtremes.maxWidth;
+
+ if (prevWord && !canBreakAfter(prevWord)) {
+ parMin += prevWordSpace + wordExtremes.minWidth;
+ } else {
+ parMin = wordExtremes.minWidth;
+ }
+
+ if (extremes->minWidth < parMin) {
+ extremes->minWidth = parMin;
+ }
+
prevWordSpace = word->origSpace;
+ prevWord = word;
//DEBUG_MSG (DEBUG_SIZE_LEVEL + 1,
// " word %s: maxWidth = %d\n",
@@ -305,19 +303,7 @@
if (extremes->maxWidth < parMax)
extremes->maxWidth = parMax;
- if (nowrap) {
- //DBG_MSGF (widget, "extremes", 0, "parMin = %d", parMin);
- if (extremes->minWidth < parMin)
- extremes->minWidth = parMin;
-
- //DEBUG_MSG (DEBUG_SIZE_LEVEL + 2,
- // " parMin = %d, after word %d (%s)\n",
- // parMin, line->last_word - 1,
- // a_Dw_content_text (&word->content));
- }
-
prevWordSpace = 0;
- parMin = 0;
parMax = 0;
}
@@ -797,7 +783,7 @@
if (lines->size () == 1) {
lastLine->top = 0;
lastLine->maxLineWidth = line1OffsetEff;
- lastLine->maxWordMin = 0;
+ lastLine->maxParMin = 0;
lastLine->maxParMax = 0;
} else {
Line *prevLine = lines->getRef (lines->size () - 2);
@@ -805,7 +791,7 @@
lastLine->top = prevLine->top + prevLine->boxAscent +
prevLine->boxDescent + prevLine->breakSpace;
lastLine->maxLineWidth = prevLine->maxLineWidth;
- lastLine->maxWordMin = prevLine->maxWordMin;
+ lastLine->maxParMin = prevLine->maxParMin;
lastLine->maxParMax = prevLine->maxParMax;
}
@@ -813,8 +799,8 @@
// lastLine->top);
//DBG_OBJ_ARRSET_NUM (page, "lines.%d.maxLineWidth", page->num_lines - 1,
// lastLine->maxLineWidth);
- //DBG_OBJ_ARRSET_NUM (page, "lines.%d.maxWordMin", page->num_lines - 1,
- // lastLine->maxWordMin);
+ //DBG_OBJ_ARRSET_NUM (page, "lines.%d.maxParMin", page->num_lines - 1,
+ // lastLine->maxParMin);
//DBG_OBJ_ARRSET_NUM (page, "lines.%d.maxParMax", page->num_lines - 1,
// lastLine->maxParMax);
//DBG_OBJ_ARRSET_NUM (page, "lines.%d.parMin", page->num_lines - 1,
@@ -847,25 +833,15 @@
//DBG_OBJ_ARRSET_NUM (page, "lines.%d.maxParMax", page->num_lines - 1,
// lastLine->maxParMax);
- /* The following code looks questionable (especially since the values
- * will be overwritten). In any case, line1OffsetEff is probably
- * supposed to go into lastLinePar*, not lastLine->par*.
- */
if (lines->size () > 1) {
- lastLine->parMin = 0;
- lastLine->parMax = 0;
+ lastLineParMax = 0;
} else {
- lastLine->parMin = line1OffsetEff;
- lastLine->parMax = line1OffsetEff;
+ lastLineParMax = line1OffsetEff;
}
- lastLineParMin = 0;
- lastLineParMax = 0;
- //DBG_OBJ_SET_NUM(page, "lastLineParMin", page->lastLineParMin);
//DBG_OBJ_SET_NUM(page, "lastLineParMax", page->lastLineParMax);
}
- lastLine->parMin = lastLineParMin;
lastLine->parMax = lastLineParMax;
//DBG_OBJ_ARRSET_NUM (page, "lines.%d.parMin", page->num_lines - 1,
@@ -887,7 +863,7 @@
Line *lastLine;
Word *word;
int availWidth, lastSpace, leftOffset, len;
- bool newLine = false, newPar = false;
+ bool newLine = false, newPar = false, canBreakBefore = true;
core::Extremes wordExtremes;
//DBG_MSGF (page, "wrap", 0, "Dw_page_real_word_wrap (%d): %s, width = %d",
@@ -942,23 +918,37 @@
/* previous word is a break */
newLine = true;
newPar = true;
- } else if (word->style->whiteSpace == core::style::WHITE_SPACE_NOWRAP ||
- word->style->whiteSpace == core::style::WHITE_SPACE_PRE) {
- //DBG_MSGF (page, "wrap", 0, "no wrap (white_space = %d)",
- // word->style->white_space);
+ } else if (!canBreakAfter (prevWord)) {
+ canBreakBefore = false;
+ // no break within nowrap
newLine = false;
newPar = false;
+ if (lastLineWidth + prevWord->origSpace + word->size.width >
+ availWidth)
+ markChange (lines->size () - 1);
} else if (lastLine->firstWord != wordIndex) {
- /* Does new word fit into the last line? */
- //DBG_MSGF (page, "wrap", 0,
- // "word %d (%s) fits? (%d + %d + %d <= %d)...",
- // word_ind, a_Dw_content_html (&word->content),
- // page->lastLine_width, prevWord->orig_space,
- // word->size.width, availWidth);
- newLine = lastLineWidth + prevWord->origSpace + word->size.width >
- availWidth;
- //DBG_MSGF (page, "wrap", 0, "... %s.",
- // newLine ? "No" : "Yes");
+ // check if we need to break because nowrap sequence is following
+ newLine = false;
+ int lineWidthNeeded = lastLineWidth + prevWord->origSpace;
+ for (int i = wordIndex; i < words->size (); i++) {
+ Word *w = words->getRef (i);
+
+ if (w->content.type == core::Content::BREAK ||
+ (word->content.type == core::Content::WIDGET &&
+ word->content.widget->blockLevel()))
+ break;
+
+ lineWidthNeeded += w->size.width;
+
+ if (lineWidthNeeded > availWidth) {
+ newLine = true;
+ break;
+ } else if (canBreakAfter (w)) {
+ break;
+ }
+
+ lineWidthNeeded += w->origSpace;
+ }
}
}
@@ -1029,27 +1019,22 @@
if (!newLine)
lastLineWidth += lastSpace;
if (!newPar) {
- lastLineParMin += lastSpace;
lastLineParMax += lastSpace;
}
lastLineWidth += word->size.width;
getWordExtremes (word, &wordExtremes);
- lastLineParMin += wordExtremes.maxWidth; /* Why maxWidth? */
lastLineParMax += wordExtremes.maxWidth;
- if (word->style->whiteSpace == core::style::WHITE_SPACE_NOWRAP ||
- word->style->whiteSpace == core::style::WHITE_SPACE_PRE) {
- lastLine->parMin += wordExtremes.minWidth + lastSpace;
+ if (!canBreakBefore) {
+ lastLineParMin += wordExtremes.minWidth + lastSpace;
/* This may also increase the accumulated minimum word width. */
- lastLine->maxWordMin =
- misc::max (lastLine->maxWordMin, lastLine->parMin);
- /* NOTE: Most code relies on that all values of nowrap are equal for all
- * words within one line. */
+ lastLine->maxParMin = misc::max (lastLine->maxParMin, lastLineParMin);
} else {
- lastLine->maxWordMin =
- misc::max (lastLine->maxWordMin, wordExtremes.minWidth);
+ lastLineParMin = wordExtremes.minWidth;
+ lastLine->maxParMin =
+ misc::max (lastLine->maxParMin, wordExtremes.minWidth);
}
//DBG_OBJ_SET_NUM(page, "lastLine_par_min", page->lastLine_par_min);
@@ -1185,6 +1170,7 @@
* the line list up from this position is rebuild. */
lines->setSize (wrapRef);
lastLineWidth = 0;
+ lastLineParMin = 0;
//DBG_OBJ_SET_NUM(page, "num_lines", page->num_lines);
//DBG_OBJ_SET_NUM(page, "lastLine_width", page->lastLine_width);
@@ -1196,7 +1182,6 @@
* to the length of the line. */
lastLine = lines->getRef (lines->size () - 1);
- lastLineParMin = lastLine->parMin;
lastLineParMax = lastLine->parMax;
wordIndex = lastLine->lastWord + 1;
@@ -1205,7 +1190,6 @@
words->getRef(i)->origSpace);
lastLineWidth += words->getRef(lastLine->lastWord)->size.width;
} else {
- lastLineParMin = 0;
lastLineParMax = 0;
wordIndex = 0;
@@ -1581,6 +1565,7 @@
word->origSpace = 0;
word->effSpace = 0;
word->content.space = false;
+ word->content.breakType = core::Content::BREAK_NO;
//DBG_OBJ_ARRSET_NUM (page, "words.%d.size.width", page->num_words - 1,
// word->size.width);
@@ -1764,6 +1749,14 @@
if (wordIndex >= 0) {
Word *word = words->getRef(wordIndex);
+ // According to http://www.w3.org/TR/CSS2/text.html#white-space-model:
+ // "line breaking opportunities are determined based on the text prior
+ // to the white space collapsing steps".
+ // So we call addBreakOption () for each Textblock::addSpace () call.
+ // This is important e.g. to be able to break between foo and bar in:
+ // <span style="white-space:nowrap">foo </span> bar
+ addBreakOption (style);
+
if (!word->content.space) {
word->content.space = true;
word->effSpace = word->origSpace = style->font->spaceWidth +
@@ -1782,7 +1775,6 @@
}
}
-
/**
* Cause a paragraph break
*/
diff --git a/dw/textblock.hh b/dw/textblock.hh
--- a/dw/textblock.hh
+++ b/dw/textblock.hh
@@ -148,11 +148,8 @@
/* The following members contain accumulated values, from the top
* down to the line before. */
int maxLineWidth; /* maximum of all line widths */
- int maxWordMin; /* maximum of all word minima */
+ int maxParMin; /* maximum of all paragraph minima */
int maxParMax; /* maximum of all paragraph maxima */
- int parMin; /* the minimal total width down from the last
- * paragraph start, to the *beginning* of the
- * line */
int parMax; /* the maximal total width down from the last
* paragraph start, to the *beginning* of the
* line */
@@ -256,6 +253,10 @@
void queueDrawRange (int index1, int index2);
void getWordExtremes (Word *word, core::Extremes *extremes);
+ inline bool canBreakAfter (Word *word)
+ {
+ return word->content.breakType == core::Content::BREAK_OK;
+ }
void markChange (int ref);
void justifyLine (Line *line, int availWidth);
Line *addLine (int wordInd, bool newPar);
@@ -382,6 +383,14 @@
void addWidget (core::Widget *widget, core::style::Style *style);
bool addAnchor (const char *name, core::style::Style *style);
void addSpace(core::style::Style *style);
+ inline void addBreakOption (core::style::Style *style)
+ {
+ int wordIndex = words->size () - 1;
+ if (wordIndex >= 0 &&
+ style->whiteSpace != core::style::WHITE_SPACE_NOWRAP &&
+ style->whiteSpace != core::style::WHITE_SPACE_PRE)
+ words->getRef(wordIndex)->content.breakType = core::Content::BREAK_OK;
+ }
void addParbreak (int space, core::style::Style *style);
void addLinebreak (core::style::Style *style);
diff --git a/dw/types.hh b/dw/types.hh
--- a/dw/types.hh
+++ b/dw/types.hh
@@ -194,11 +194,16 @@
REAL_CONTENT = 0xff ^ (START | END),
SELECTION_CONTENT = TEXT | WIDGET | BREAK
};
+ enum BreakType {
+ BREAK_NO,
+ BREAK_OK
+ };
/* Content is embedded in struct Word therefore we
* try to be space efficient.
*/
short type;
bool space;
+ unsigned char breakType;
union {
const char *text;
Widget *widget;
diff --git a/src/html.cc b/src/html.cc
--- a/src/html.cc
+++ b/src/html.cc
@@ -1107,6 +1107,8 @@
}
if (spaceCnt) {
+ // add break possibility for the white-space:pre-wrap case
+ HT2TB(html)->addBreakOption (html->styleEngine->wordStyle ());
spc = dStrnfill(spaceCnt, ' ');
HT2TB(html)->addText (spc, spaceCnt, html->styleEngine->wordStyle ());
dFree(spc);
@@ -1208,10 +1210,12 @@
Html_process_space(html, word2 + start, i - start);
} else if (!strncmp(word2+i, utf8_zero_width_space, 3)) {
i += 3;
+ HT2TB(html)->addBreakOption(html->styleEngine->wordStyle ());
} else if (a_Utf8_ideographic(word2+i, beyond_word2, &len)) {
i += len;
HT2TB(html)->addText(word2 + start, i - start,
html->styleEngine->wordStyle ());
+ HT2TB(html)->addBreakOption(html->styleEngine->wordStyle ());
} else {
do {
i += len;