Currently, when a per-script font setting is not set, we fall back to the default, common script font setting. For example, standardFontFamily(USCRIPT_ARABIC) will return standardFontFamily(USCRIPT_COMMON) if setStandardFontFamily("Font", USCRIPT_ARABIC) was never called. However, there is no way to unset a font setting, so once a per-script font setting is set, it can never fallback to the common script font setting. It'd be useful to allow this, so it can be possible for a user to clear their font setting for Arabic script and just use their setting for the default script. One way to do this is by treating the empty string the same as an unset setting.
Created attachment 132242 [details] Patch
Comment on attachment 132242 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=132242&action=review > Source/WebCore/page/Settings.cpp:67 > - if (it != fontMap.end()) > + if (it != fontMap.end() && !it->second.isEmpty()) This is ok, but can we remove the map entry when an empty font family name is set? It will reduce memory usage.
Created attachment 132544 [details] Patch
(In reply to comment #2) > (From update of attachment 132242 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=132242&action=review > > > Source/WebCore/page/Settings.cpp:67 > > - if (it != fontMap.end()) > > + if (it != fontMap.end() && !it->second.isEmpty()) > > This is ok, but can we remove the map entry when an empty font family name is set? It will reduce memory usage. Thanks, uploaded revised patch.
Comment on attachment 132544 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=132544&action=review > Source/WebCore/page/Settings.cpp:71 > + if (family.isEmpty()) { > + if (it != fontMap.end()) { > + fontMap.remove(it); > + changed = true; > + } > + } else if (it == fontMap.end() || it->second != family) { > + fontMap.set(static_cast<int>(script), family); > + changed = true; > + } > + > + if (changed) > + page->setNeedsRecalcStyleInAllFrames(); The variable 'changed' is not needed. We prefer early-return. The code should be: if (family.isEmpty()) { if (it == fontMap.end()) return; fontMap.remove(it); } else if (it != fontMap.end() && it->second == family) return; else fontMap.set(...); page->setNeedsRecalcStyleInAllFrames();
Created attachment 132545 [details] Patch
Comment on attachment 132545 [details] Patch ok
Comment on attachment 132545 [details] Patch Clearing flags on attachment: 132545 Committed r111157: <http://trac.webkit.org/changeset/111157>
All reviewed patches have been landed. Closing bug.